feat(backend): add billing cadence and pro-rating config to plan and subscription #2890
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (79)
📝 WalkthroughWalkthroughThis change introduces explicit support for billing cadence and pro-rating configuration in both plan and subscription models across the API, database schema, domain logic, and test suites. New fields for billing cadence (as ISO8601 duration) and pro-rating configuration are added, with corresponding validation, migration, and propagation throughout all relevant layers. Changes
Possibly related PRs
Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
tothandras
force-pushed
the
feat/plan-billing-cadence
branch
2 times, most recently
from
ecad245 to
e44250f
Compare
tothandras
changed the title
tothandras
force-pushed
the
feat/plan-billing-cadence
branch
3 times, most recently
from
bde3f73 to
ad30221
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: 12
🔭 Outside diff range comments (1)
openmeter/productcatalog/plan/service.go (1)
222-261: 🛠️ Refactor suggestionConsider adding validation for new billing configuration fields.
The
Validate()method doesn't include validation logic for the newBillingCadenceandProRatingConfigfields. Consider adding validation to ensure:
- Billing cadence format is valid when provided
- Pro-rating configuration values are within acceptable ranges
- Consistency between billing cadence and other plan settings
Example validation addition:
if i.Phases != nil { for _, phase := range *i.Phases { if err := phase.Validate(); err != nil { errs = append(errs, fmt.Errorf("invalid plan phase: %w", err)) } } } + + if i.BillingCadence != nil { + if err := i.BillingCadence.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid billing cadence: %w", err)) + } + } + + if i.ProRatingConfig != nil { + if err := i.ProRatingConfig.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid pro-rating config: %w", err)) + } + }
♻️ Duplicate comments (5)
api/openapi.cloud.yaml (5)
19202-19218:billingCadence/proRatingConfigdefinitions duplicated
This block repeats the property definitions already flagged for refactoring. Please apply the shared-component approach suggested above.
19457-19473:billingCadence/proRatingConfigdefinitions duplicated
Another instance of duplicated schema definitions—please consolidate by referencing the shared component.
19632-19648:billingCadence/proRatingConfigdefinitions duplicated
Please reuse the shared schemas here instead of redefining.
20935-20953:billingCadence/proRatingConfigdefinitions duplicated
Yet another repeat of the full property definitions—please reference the shared components to DRY up the spec.
21407-21425:billingCadence/proRatingConfigdefinitions duplicated
Apply the shared-component refactor here as well to avoid lengthy repeats.
🧹 Nitpick comments (10)
api/spec/src/productcatalog/prorating.tsp (1)
7-12: Consider the design intent for single-member union.The
ProRatingModeunion currently contains only one member (proratePrices). While this works, it's unusual to define a union for a single value. This suggests either:
- Additional pro-rating modes are planned for future releases
- This could be simplified to a string literal or enum
If this is intentional for future extensibility, consider adding a comment explaining the design intent.
openmeter/productcatalog/plan.go (1)
213-217: Consider consistent error message formatting.The new fields are well-documented and properly validated.
For consistency with other validation errors in the codebase, consider standardizing the error format:
if p.BillingCadence.IsZero() { - errs = append(errs, fmt.Errorf("invalid BillingCadence: must not be empty")) + errs = append(errs, fmt.Errorf("invalid billing cadence: must not be empty")) } if err := p.ProRatingConfig.Validate(); err != nil { - errs = append(errs, fmt.Errorf("invalid ProRatingConfig: %s", err)) + errs = append(errs, fmt.Errorf("invalid pro-rating config: %w", err)) }Note: Using
%winstead of%spreserves error wrapping for better error tracing.Also applies to: 243-249
openmeter/productcatalog/plan/httpdriver/mapping.go (1)
155-169: Verify the default pro-rating configuration aligns with business requirements.The function correctly handles nil input by providing defaults (enabled=true, mode=ProRatingModeProratePrices). However, automatically enabling pro-rating by default could have significant billing implications.
Consider:
- Confirming these defaults with product requirements
- Documenting why these specific defaults were chosen
- Potentially extracting defaults to named constants for better maintainability
+// Default pro-rating configuration values +const ( + defaultProRatingEnabled = true + defaultProRatingMode = productcatalog.ProRatingModeProratePrices +) + // asProRatingConfig converts API ProRatingConfig to domain ProRatingConfig func asProRatingConfig(p *api.ProRatingConfig) productcatalog.ProRatingConfig { if p == nil { // Return default configuration when not provided return productcatalog.ProRatingConfig{ - Enabled: true, - Mode: productcatalog.ProRatingModeProratePrices, + Enabled: defaultProRatingEnabled, + Mode: defaultProRatingMode, } }api/openapi.yaml (4)
13097-13113: DRY Refactor: ExtractbillingCadenceandproRatingConfigdefinitions
The inline definitions are repeated across schemas—consider defining reusable components (e.g.,BillingCadence) and referencing them via$ref. Also verify that tooling supports the customformat: duration.
19850-19869: DRY Refactor: CentralizebillingCadenceandproRatingConfigschema definitions
These property definitions are duplicated—consider creating dedicated component schemas and referencing them via$ref. Also confirm that default values align with service defaults.
20025-20044: DRY Refactor: Reuse commonbillingCadenceandproRatingConfigdefinitions
To reduce duplication, factor out these property schemas into components and reference them. Also ensure API consumers handle these additions gracefully.
21466-21487: Nitpick: Align descriptions for subscription-levelbillingCadence
The wording differs slightly from plan schemas—consider standardizing the description to maintain consistency across the API.tools/migrate/productcatalog_test.go (1)
217-588: Consider extracting test data setup into helper functions.The test setup is quite lengthy. For improved readability and maintainability, consider extracting the creation of plans, phases, and rate cards into helper functions.
Example helper function:
func createTestPlan(t *testing.T, db *sql.DB, id, key, name string) { _, err := db.Exec(` INSERT INTO plans (namespace, id, created_at, updated_at, key, version, name, currency) VALUES ('default', $1, NOW(), NOW(), $2, 1, $3, 'USD')`, id, key, name, ) require.NoError(t, err) } func createTestPhase(t *testing.T, db *sql.DB, id, planId, key, name string, index int, duration *string) { durationValue := "NULL" if duration != nil { durationValue = fmt.Sprintf("'%s'", *duration) } query := fmt.Sprintf(` INSERT INTO plan_phases (namespace, id, created_at, updated_at, key, name, plan_id, index, duration) VALUES ('default', '%s', NOW(), NOW(), '%s', '%s', '%s', %d, %s)`, id, key, name, planId, index, durationValue, ) _, err := db.Exec(query) require.NoError(t, err) }api/client/javascript/src/zod/index.ts (1)
11070-11071: Consider documenting the single enum value intention.The
modeenum currently only has one value'prorate_prices'. If this is intentional for the initial implementation with plans to expand later, consider adding a comment to clarify this design decision.Add a comment to clarify the single enum value:
mode: zod .enum(['prorate_prices']) // Additional modes may be added in future iterations .describe( 'Pro-rating mode options for handling billing period changes.' )Also applies to: 12097-12098, 14861-14862, 17348-17349
api/openapi.cloud.yaml (1)
13187-13203: Refactor repeated property definitions into reusable components
The full definitions ofbillingCadenceandproRatingConfigare duplicated across multiple schemas. Consider extracting these into shared components under#/components/schemas—for example, aBillingCadenceschema and reuse it via$ref. This will reduce duplication and simplify future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (66)
api/client/javascript/src/client/schemas.ts(8 hunks)api/client/javascript/src/zod/index.ts(12 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/package.json(1 hunks)api/spec/src/productcatalog/plan.tsp(2 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)e2e/productcatalog_test.go(6 hunks)openmeter/billing/worker/subscription/sync_test.go(7 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(16 hunks)openmeter/ent/db/plan.go(5 hunks)openmeter/ent/db/plan/plan.go(5 hunks)openmeter/ent/db/plan/where.go(3 hunks)openmeter/ent/db/plan_create.go(11 hunks)openmeter/ent/db/plan_update.go(7 hunks)openmeter/ent/db/predicate/predicate.go(2 hunks)openmeter/ent/db/runtime.go(2 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(5 hunks)openmeter/ent/db/subscription/where.go(3 hunks)openmeter/ent/db/subscription_create.go(12 hunks)openmeter/ent/db/subscription_update.go(7 hunks)openmeter/ent/schema/productcatalog.go(3 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/adapter/feature_test.go(1 hunks)openmeter/productcatalog/addon/adapter/mapping.go(2 hunks)openmeter/productcatalog/errors.go(1 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/plan/adapter/mapping.go(2 hunks)openmeter/productcatalog/plan/adapter/plan.go(2 hunks)openmeter/productcatalog/plan/httpdriver/mapping.go(6 hunks)openmeter/productcatalog/plan/serializer_test.go(1 hunks)openmeter/productcatalog/plan/service.go(3 hunks)openmeter/productcatalog/plan/service/plan.go(1 hunks)openmeter/productcatalog/plan/service/service_test.go(1 hunks)openmeter/productcatalog/plan_test.go(13 hunks)openmeter/productcatalog/planaddon_test.go(3 hunks)openmeter/productcatalog/pro_rating.go(1 hunks)openmeter/productcatalog/subscription/http/mapping.go(4 hunks)openmeter/productcatalog/subscription/plan.go(1 hunks)openmeter/productcatalog/subscription/service/plan.go(1 hunks)openmeter/productcatalog/testutils/plan.go(2 hunks)openmeter/subscription/addon/diff/restore_test.go(2 hunks)openmeter/subscription/aligment_test.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(1 hunks)openmeter/subscription/repository.go(1 hunks)openmeter/subscription/subscription.go(2 hunks)openmeter/subscription/subscriptionspec.go(3 hunks)openmeter/subscription/subscriptionview.go(1 hunks)openmeter/subscription/testutils/plan.go(1 hunks)openmeter/subscription/workflow/service/addon_test.go(2 hunks)openmeter/subscription/workflow/service/subscription.go(1 hunks)openmeter/subscription/workflow/service/subscription_test.go(2 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)test/billing/subscription_test.go(2 hunks)test/customer/customer.go(3 hunks)test/subscription/scenario_editaligned_test.go(3 hunks)test/subscription/scenario_editcancel_test.go(2 hunks)tools/migrate/entitlement_test.go(5 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql(1 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql(1 hunks)tools/migrate/productcatalog_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (26)
openmeter/subscription/testutils/plan.go (2)
pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/addon/diff/restore_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/testutils/plan.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)api/api.gen.go (3)
Metadata(4629-4629)ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/repo/mapping.go (5)
openmeter/ent/db/subscription.go (2)
Subscription(22-60)Subscription(129-148)openmeter/ent/schema/subscription.go (5)
Subscription(18-20)Subscription(22-31)Subscription(33-57)Subscription(59-64)Subscription(66-77)openmeter/subscription/subscription.go (1)
Subscription(13-32)openmeter/subscription/plan.go (1)
PlanRef(11-15)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
test/customer/customer.go (2)
pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/adapter/feature_test.go (1)
openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/errors.go (2)
pkg/models/errors.go (1)
ErrorCode(294-294)pkg/models/validationissue.go (3)
NewValidationIssue(219-230)WithFieldString(196-202)WithWarningSeverity(214-216)
openmeter/subscription/aligment_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/workflow/service/addon_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/repo/subscriptionrepo.go (7)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (1)
ProRatingConfig(5852-5858)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)api/client/go/client.gen.go (1)
ProRatingConfig(5382-5388)
openmeter/productcatalog/subscription/service/plan.go (2)
pkg/models/errors.go (1)
ErrorWithFieldPrefix(340-346)pkg/models/fieldselector.go (2)
NewFieldSelectors(229-231)NewFieldSelector(177-179)
openmeter/subscription/workflow/service/subscription.go (1)
pkg/models/errors.go (1)
NewGenericValidationError(138-140)
test/subscription/scenario_editcancel_test.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/service/plan.go (4)
openmeter/ent/db/plan/where.go (6)
Key(100-102)Version(110-112)Name(90-92)Description(95-97)Currency(115-117)BillingCadence(120-123)pkg/models/metadata.go (1)
Metadata(9-9)openmeter/productcatalog/alignment.go (1)
Alignment(3-6)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/productcatalog/plan/service/service_test.go (4)
openmeter/productcatalog/planaddon/service/service_test.go (1)
MonthPeriod(25-25)openmeter/productcatalog/addon/adapter/adapter_test.go (1)
MonthPeriod(25-25)openmeter/productcatalog/plan/adapter/adapter_test.go (1)
MonthPeriod(22-22)pkg/isodate/date.go (1)
MustParse(183-190)
openmeter/productcatalog/plan/service.go (2)
pkg/isodate/date.go (1)
Period(47-49)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
e2e/productcatalog_test.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/billing/worker/subscription/sync_test.go (7)
openmeter/ent/db/plan/where.go (2)
Currency(115-117)BillingCadence(120-123)openmeter/ent/db/subscription/where.go (2)
Currency(126-129)BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)api/client/javascript/src/client/schemas.ts (2)
Currency(10573-10573)ProRatingConfig(10915-10915)api/api.gen.go (3)
Currency(1876-1888)ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/ent/db/runtime.go (4)
openmeter/ent/db/plan/plan.go (2)
DefaultProRatingConfig(130-130)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (2)
DefaultProRatingConfig(148-148)ValueScanner(152-154)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/ent/db/subscription.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/subscription/subscription.go (3)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)ValueScanner(152-154)
api/client/javascript/src/client/schemas.ts (3)
api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingMode(5861-5861)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingMode(14-14)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingMode(5391-5391)
openmeter/ent/schema/subscription.go (4)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)openmeter/ent/schema/productcatalog.go (1)
ProRatingConfigValueScanner(200-200)
openmeter/productcatalog/pro_rating.go (5)
api/client/javascript/src/client/schemas.ts (2)
ProRatingMode(10916-10916)ProRatingConfig(10915-10915)api/api.gen.go (3)
ProRatingMode(5861-5861)ProRatingModeProratePrices(700-700)ProRatingConfig(5852-5858)api/client/go/client.gen.go (3)
ProRatingMode(5391-5391)ProRatingModeProratePrices(637-637)ProRatingConfig(5382-5388)pkg/models/validator.go (1)
Validate(16-26)pkg/models/errors.go (1)
NewNillableGenericValidationError(129-135)
openmeter/ent/db/plan.go (6)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/planratecard/where.go (1)
BillingCadence(111-114)pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/plan.go (14)
FieldID(18-18)FieldNamespace(20-20)FieldName(30-30)FieldDescription(32-32)FieldKey(34-34)FieldCurrency(40-40)FieldBillingCadence(42-42)FieldCreatedAt(24-24)FieldUpdatedAt(26-26)FieldDeletedAt(28-28)FieldEffectiveFrom(46-46)FieldEffectiveTo(48-48)FieldProRatingConfig(44-44)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (11)
FieldID(18-18)FieldNamespace(20-20)FieldName(36-36)FieldDescription(38-38)FieldCurrency(44-44)FieldBillingCadence(46-46)FieldCreatedAt(22-22)FieldUpdatedAt(24-24)FieldDeletedAt(26-26)FieldProRatingConfig(48-48)ValueScanner(152-154)
openmeter/ent/db/mutation.go (5)
pkg/isodate/date.go (1)
String(16-16)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/plan.go (2)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (2)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)
openmeter/ent/db/subscription_create.go (9)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/where.go (2)
BillingCadence(120-123)Name(90-92)openmeter/ent/db/subscription/where.go (2)
BillingCadence(132-135)Name(106-108)openmeter/ent/db/ent.go (2)
ValidationError(255-258)Value(65-65)pkg/models/validator.go (1)
Validate(16-26)openmeter/ent/db/subscription.go (2)
Subscription(22-60)Subscription(129-148)openmeter/ent/schema/subscription.go (5)
Subscription(18-20)Subscription(22-31)Subscription(33-57)Subscription(59-64)Subscription(66-77)openmeter/subscription/subscription.go (1)
Subscription(13-32)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Analyze (go)
🔇 Additional comments (172)
api/spec/package.json (1)
29-29: LGTM! Package manager update looks good.The pnpm version update from 10.7.0 to 10.11.0 is properly implemented with the corresponding SHA512 hash update.
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql (1)
1-4: LGTM! Migration rollback script is correctly implemented.The down migration properly drops the
pro_rating_configandbilling_cadencecolumns from bothplansandsubscriptionstables, effectively reversing the schema changes introduced in the corresponding up migration.openmeter/productcatalog/adapter/feature_test.go (1)
322-326: LGTM! Test updates properly include new required fields.The test correctly sets the new
BillingCadenceandProRatingConfigfields when creating a plan:
BillingCadence("P1M")uses a valid ISO8601 duration for 1 monthProRatingConfigis properly configured withEnabled: trueandMode: ProRatingModeProratePricesThese additions ensure the test remains functional with the new schema requirements.
openmeter/subscription/subscription.go (1)
9-9: LGTM! Import addition is necessary and correct.The
isodatepackage import is required for the newBillingCadencefield of typeisodate.Period.openmeter/subscription/aligment_test.go (1)
36-40: LGTM! Proper initialization of new plan fields.The addition of
BillingCadenceandProRatingConfigfields aligns with the broader system changes. The usage ofisodate.MustParse(t, "P1M")for monthly billing cadence and enabling pro-rating with price proration mode is appropriate for this billing alignment test.openmeter/productcatalog/plan/serializer_test.go (1)
37-41: LGTM! Ensures serialization coverage for new fields.The addition of
BillingCadenceandProRatingConfigfields to the serialization test ensures that these new plan metadata fields are properly tested for JSON marshaling/unmarshaling. The field initialization is consistent with other test files.test/billing/subscription_test.go (2)
115-123: LGTM! Note the daily billing cadence choice.The addition of
BillingCadenceandProRatingConfigfields is correct. The use of daily billing cadence ("P1D") instead of monthly ("P1M") used in other tests might be intentional for this billing test scenario, possibly to accelerate test execution or match specific requirements.
166-174: LGTM! Consistent with the paid plan configuration.The free plan correctly receives the same billing cadence and pro-rating configuration as the paid plan, maintaining consistency within this test scenario.
test/subscription/scenario_editaligned_test.go (3)
23-23: LGTM! Proper import addition.The addition of the
isodateimport is necessary for theisodate.MustParsecalls used in the test plan configurations.
57-64: LGTM! Appropriate configuration for aligned subscription test.The addition of
BillingCadenceandProRatingConfigfields to the aligned subscription test plan is correct. The monthly billing cadence and enabled pro-rating configuration will properly test the billing alignment logic.
238-245: LGTM! Consistent configuration for unaligned subscription test.The unaligned subscription test plan receives the same billing cadence and pro-rating configuration, which is appropriate for testing the different behavior when
BillablesMustAlignis set to false.openmeter/subscription/addon/diff/restore_test.go (2)
23-23: LGTM: Import addition is appropriate.The
isodatepackage import is correctly added to support theisodate.MustParsefunction used in the test data.
354-358: LGTM: Billing cadence and pro-rating configuration properly added.The test data is appropriately updated to include the new billing cadence and pro-rating configuration fields:
- Monthly billing cadence (
P1M) is a reasonable default for testing- Pro-rating is enabled with the
ProRatingModeProratePricesmode, which aligns with the imported constantsThis ensures the test reflects the enhanced domain model.
openmeter/subscription/workflow/service/addon_test.go (2)
27-27: LGTM: Import addition is necessary and correct.The
isodatepackage import is properly added to support the billing cadence parsing functionality.
571-575: LGTM: Consistent field initialization for billing cadence and pro-rating.The billing cadence and pro-rating configuration fields are properly initialized with appropriate test values:
- Monthly billing cadence maintains consistency across test files
- Pro-rating configuration with enabled state and prorate prices mode is correctly set
This ensures test scenarios reflect the new billing and pro-rating capabilities.
openmeter/productcatalog/subscription/plan.go (1)
74-77: LGTM: Proper field propagation in production code.The
ToCreateSubscriptionPlanInputmethod correctly includes the newBillingCadenceandProRatingConfigfields in the returnedCreateSubscriptionPlanInputstruct. This ensures that billing cadence and pro-rating configuration from the plan are properly propagated to subscription creation inputs.The implementation follows the existing pattern of direct field mapping and maintains consistency with the broader PR changes.
test/customer/customer.go (3)
22-22: LGTM: Required import for billing cadence parsing.The
isodatepackage import is correctly added to support theisodate.MustParsefunction used in test plan definitions.
226-232: LGTM: Test plan enhanced with billing and pro-rating configuration.The
emptyExamplePlaninTestUpdateWithSubscriptionPresentis properly updated to include:
- Monthly billing cadence (
P1M) using the appropriate parsing function- Pro-rating configuration with enabled state and prorate prices mode
This ensures customer tests work with the enhanced plan model.
561-567: LGTM: Consistent test plan configuration in deletion test.The second
emptyExamplePlandefinition inTestDeleteis identically configured with the same billing cadence and pro-rating settings, maintaining consistency across test scenarios.openmeter/subscription/repo/mapping.go (2)
31-35: LGTM! Proper type conversion and error handling.The implementation correctly parses the billing cadence from
isodate.String(database representation) toisodate.Period(domain representation) with appropriate error handling.
61-62: Excellent field mapping.The new fields are correctly mapped to the domain subscription struct, maintaining consistency with the broader schema changes.
openmeter/subscription/subscriptionview.go (1)
241-246: Clean field mapping implementation.The subscription view correctly includes the new billing cadence and pro-rating configuration fields from the subscription object, maintaining consistency with the enhanced subscription specification.
openmeter/subscription/workflow/service/subscription_test.go (2)
28-28: Good addition of required import.The
isodateimport is correctly added to support the billing cadence period parsing functionality used in the test data.
863-871: Well-structured test data enhancement.The test plan metadata is appropriately enhanced with realistic values:
- One-month billing cadence (
P1M) for testing monthly billing scenarios- Pro-rating enabled with price proration mode for comprehensive feature testing
openmeter/subscription/testutils/plan.go (1)
59-67: Excellent test utility defaults.The default billing cadence and pro-rating configuration provide comprehensive test coverage:
- One-month billing cadence using the safe
MustParsehelper- Pro-rating enabled with price proration mode for testing discount scenarios
- Values are consistent across test files and follow established patterns
openmeter/subscription/repository.go (1)
30-34: LGTM! Clean addition of billing cadence and pro-rating configuration fields.The new fields are well-structured with:
- Appropriate field types (
isodate.Periodfor durations,productcatalog.ProRatingConfigfor configuration)- Consistent JSON tag naming convention
- Clear documentation comments
- Alignment with the broader system design for billing cadence support
openmeter/productcatalog/plan/service/service_test.go (1)
28-30: Excellent refactor to use precise ISO 8601 durations.This change improves the test accuracy by:
- Replacing fixed 30-day approximations with exact monthly periods (
P1M)- Using
isodate.MustParse()for proper ISO 8601 duration parsing- Moving from package-level to local variables for better test isolation
- Aligning with the system-wide adoption of precise billing cadence handling
openmeter/productcatalog/planaddon_test.go (1)
46-54: Good test coverage for the new billing cadence and pro-rating fields.The test cases are consistently updated to include:
BillingCadenceset to monthly periods (P1M) using proper ISO 8601 parsingProRatingConfigwith enabled pro-rating andProRatingModeProratePrices- Coverage across all validation scenarios (valid, invalid, incompatible)
This ensures the validation logic properly handles the new fields introduced in the PR.
Also applies to: 214-222, 409-417
test/subscription/scenario_editcancel_test.go (1)
24-24: Well-integrated addition of billing cadence and pro-rating configuration to the test plan.The changes are clean and consistent:
- Proper import of
isodatepackage for duration handling- Plan metadata enhanced with
BillingCadence: isodate.MustParse(t, "P1M")for monthly billingProRatingConfigconfigured with enabled pro-rating andProRatingModeProratePrices- Values align with the established patterns throughout the codebase
This ensures the subscription editing and canceling scenarios properly test the new billing cadence functionality.
Also applies to: 58-65
openmeter/productcatalog/plan_test.go (3)
13-13: LGTM: Import addition is appropriate.The
isodatepackage import is correctly added to support the newBillingCadencefield usage.
33-37: LGTM: Consistent test data enhancement.The addition of
BillingCadenceandProRatingConfigfields to all test cases is well-structured and consistent:
- BillingCadence: Using "P1M" (1 month) is a reasonable default for testing
- ProRatingConfig: Enabling pro-rating with
ProRatingModeProratePricesprovides good test coverageThe changes maintain test clarity while ensuring all plan metadata includes the new required fields.
Also applies to: 50-54, 67-71, 84-88, 101-105, 118-122, 135-139, 152-156, 169-173
197-203: LGTM: Alignment test coverage updated correctly.The
TestAlignmentEnforcementtest cases have been properly updated to include the new billing cadence and pro-rating configuration fields, ensuring comprehensive validation of plan alignment rules with the new metadata.Also applies to: 252-258, 307-313
openmeter/productcatalog/plan/service/plan.go (1)
754-762: LGTM: Enhanced plan versioning with comprehensive metadata preservation.The addition of
Alignment,BillingCadence, andProRatingConfigto the field copying logic is excellent. When creating a new version of a plan, preserving these configuration settings ensures continuity and expected behavior:
- Alignment: Billing alignment rules should carry forward
- BillingCadence: Default billing frequency should be preserved
- ProRatingConfig: Pro-rating settings should remain consistent
This follows the established pattern of copying relevant metadata from the source plan.
openmeter/productcatalog/testutils/plan.go (2)
11-11: LGTM: Proper import for ISO date functionality.The
isodatepackage import is correctly added to support the billing cadence parsing.
29-33: LGTM: Well-structured test utility enhancement.The addition of
BillingCadenceandProRatingConfigto the test plan utility provides excellent defaults:
- BillingCadence: "P1M" (monthly) is a reasonable default for test scenarios
- ProRatingConfig: Enabling pro-rating with price proration mode ensures test coverage of the feature
These defaults align with values used in other test files, maintaining consistency across the test suite.
openmeter/productcatalog/subscription/service/plan.go (1)
63-64: LGTM: Enhanced error reporting with structured field context.The addition of
models.ErrorWithFieldPrefiximproves error handling by:
- Clear context: Validation errors now include "plan" field prefix for better debugging
- Structured reporting: Follows established patterns for hierarchical error reporting
- Developer experience: Makes it easier to identify which part of the input caused validation failures
This enhancement aligns well with structured error handling best practices.
openmeter/productcatalog/plan/adapter/mapping.go (2)
17-20: LGTM! Good error handling for billing cadence parsing.The billing cadence parsing includes proper error handling with a descriptive error message that includes the invalid value. This follows good error handling practices.
39-40: LGTM! Clean integration of new fields into domain model.The new
BillingCadenceandProRatingConfigfields are properly integrated into thePlanMetastruct, maintaining consistency with the existing field assignments.openmeter/subscription/workflow/service/subscription.go (1)
144-151: LGTM! Proper billing cadence alignment validation.The billing cadence alignment validation is correctly positioned after applying customizations but before updating the subscription. The error handling properly distinguishes between validation errors and alignment failures, using appropriate error types.
openmeter/ent/db/predicate/predicate.go (1)
215-224: LGTM! Generated code follows consistent patterns.The new
PlanOrErrandSubscriptionOrErrfunctions follow the exact same pattern as existing*OrErrfunctions in this generated file, providing consistent error handling for predicate composition.Also applies to: 249-258
openmeter/subscription/repo/subscriptionrepo.go (1)
106-107: LGTM! Proper persistence of new subscription fields.The new
BillingCadenceandProRatingConfigfields are correctly added to the subscription creation command. The use of.ISOString()for billing cadence is appropriate for database storage, and the direct assignment ofProRatingConfigaligns with the schema design.openmeter/productcatalog/errors.go (1)
462-470: LGTM! Well-structured error definitions for billing cadence validation.The new error code and validation issue follow the established patterns in the codebase. The warning severity is appropriate for billing cadence compatibility validation, and the field targeting is correctly set to "billingCadence".
openmeter/productcatalog/plan/adapter/plan.go (2)
183-184: LGTM! Proper handling of new fields in plan creation.The billing cadence is correctly converted to ISO string format using
ISOString(), and the pro-rating config is set directly. This follows the established pattern for required fields in the create operation.
428-430: LGTM! Consistent approach for nullable updates.The use of
SetNillableBillingCadenceandSetNillableProRatingConfigis appropriate for update operations, allowing these fields to be optionally modified. TheISOStringPtrOrNil()method properly handles the conversion for nullable billing cadence.openmeter/productcatalog/addon/adapter/mapping.go (2)
187-190: LGTM! Proper error handling for billing cadence parsing.The billing cadence parsing includes appropriate error handling with a descriptive error message that includes the field name for debugging context.
207-208: LGTM! Correct assignment of new fields to plan metadata.Both
BillingCadenceandProRatingConfigare properly assigned to thePlanMetastructure, maintaining consistency with the domain model.pkg/isodate/date_test.go (1)
81-217: LGTM! Comprehensive test coverage for period divisibility.The
TestDivisibleByfunction provides excellent test coverage with:
- Well-structured table-driven tests
- Comprehensive coverage of compatible period combinations (years/months, months/weeks, etc.)
- Good coverage of incompatible scenarios
- Proper error handling validation
- Descriptive test case names that clearly communicate the expected behavior
This test suite will help ensure the billing cadence validation logic works correctly across different period combinations.
api/spec/src/productcatalog/prorating.tsp (1)
18-30: Well-structured pro-rating configuration model.The
ProRatingConfigmodel is well-designed with:
- Clear field descriptions and summaries
- Sensible defaults (enabled: true, mode: proratePrices)
- Proper TypeSpec annotations
api/spec/src/productcatalog/subscription.tsp (2)
95-104: Well-implemented required billing cadence field.The
billingCadencefield is properly implemented with:
- Appropriate visibility settings (read-only)
- ISO8601 duration encoding
- Clear documentation and example
- Required field status which ensures all subscriptions have billing frequency defined
106-114: Correctly implemented pro-rating configuration.The
proRatingConfigfield implementation is solid:
- Optional field with sensible default values
- Consistent with the plan model design
- Proper TypeSpec object literal syntax using
#{}- Read-only visibility maintains data integrity
api/spec/src/productcatalog/plan.tsp (2)
83-92: Excellent implementation of billing cadence for plans.The
billingCadencefield is well-implemented with:
- Required field ensuring all plans define billing frequency
- Comprehensive visibility settings (Read, Create, Update)
- Proper ISO8601 duration encoding
- Clear documentation with examples
- Consistent design with the subscription model
94-102: Consistent pro-rating configuration implementation.The
proRatingConfigfield maintains excellent consistency with the subscription model:
- Same default values ensuring predictable behavior
- Appropriate visibility settings for plan management
- Optional field allows flexibility while providing sensible defaults
- Clear documentation and summary
This consistency between Plan and Subscription models will help prevent configuration mismatches.
openmeter/billing/worker/subscription/sync_test.go (7)
157-165: LGTM! Consistent addition of billing cadence and pro-rating configuration.The addition of
BillingCadenceandProRatingConfigfields to thePlanMetastruct aligns with the PR objectives. The values used are appropriate:
BillingCadence: isodate.MustParse(s.T(), "P1M")sets a monthly billing cycleProRatingConfigis properly configured with enabled pro-rating using price proration mode
574-582: LGTM! Consistent configuration for pro-rating test scenario.The billing cadence and pro-rating configuration are appropriately set for the
TestInArrearsProratingtest case. The configuration matches the test's purpose of validating pro-rating behavior.
1391-1395: LGTM! Proper configuration for zero-price testing.The addition of billing cadence and pro-rating configuration is consistent with other test cases. Even though this test focuses on zero prices, having these fields properly configured ensures comprehensive test coverage.
1467-1471: LGTM! Appropriate billing cadence for alignment testing.The billing cadence is set to "P2W" (2 weeks) which is different from other tests that use "P1M" (1 month). This variation is appropriate for testing aligned subscription invoicing scenarios and provides good test coverage of different billing periods.
1690-1694: LGTM! Consistent configuration for cancellation scenario.The billing cadence and pro-rating configuration are properly set for testing subscription cancellation behavior. The monthly cadence and enabled pro-rating will ensure proper testing of cancellation-related pro-rating calculations.
1859-1863: LGTM! Appropriate configuration for progressive billing test.The billing cadence and pro-rating configuration are correctly configured for the progressive billing cancellation test scenario. This ensures the test properly validates pro-rating behavior during progressive billing workflows.
3805-3814: LGTM! Consistent default configuration in helper function.The addition of billing cadence and pro-rating configuration to the
createSubscriptionFromPlanPhaseshelper function ensures that all tests using this helper will have consistent default values for the new fields. This is a good practice for maintaining test consistency.openmeter/ent/schema/subscription.go (1)
40-55: Well-structured schema additions for billing cadence and pro-rating configuration.The new fields are properly defined with:
- Appropriate Go types (
isodate.Stringfor billing cadence,productcatalog.ProRatingConfigfor pro-rating)- Custom value scanner for JSON serialization of the pro-rating config
- Sensible default values (pro-rating enabled with
ProRatingModeProratePrices)- JSONB storage type for PostgreSQL
- Clear documentation comments
The schema changes integrate well with the existing codebase structure and follow established patterns.
openmeter/productcatalog/plan/service.go (3)
13-13: Good addition of isodate import.The import is necessary for the new
BillingCadencefield and follows the existing import organization.
155-159: Well-designed optional fields for billing configuration.The new fields are properly structured as optional pointers, allowing for selective updates. The type choices (
*isodate.Periodfor billing cadence and*productcatalog.ProRatingConfigfor pro-rating) are appropriate for the business logic layer.
199-205: Correct equality comparison logic for new fields.The comparison logic properly handles:
- String representation comparison for
BillingCadenceusing.String()method- Structural equality for
ProRatingConfigusing.Equal()methodThis ensures accurate change detection in the update input.
openmeter/ent/db/subscription.go (5)
16-19: Correct imports for new field types.The imports for
productcatalogandisodatepackages are properly added to support the new field types.
52-55: Well-structured field definitions with proper types and documentation.The new fields are correctly defined with:
BillingCadenceasisodate.Stringwith appropriate commentProRatingConfigasproductcatalog.ProRatingConfigwith descriptive commentThe field types match the schema definitions and follow established patterns.
137-142: Correct scanning setup for new fields.The
scanValuesmethod properly handles:
billing_cadenceas string field (line 137)pro_rating_configusing the custom value scanner (lines 141-142)This ensures proper data retrieval from the database.
248-259: Proper value assignment logic for new fields.The assignment logic correctly:
- Converts
sql.NullStringtoisodate.Stringfor billing cadence (lines 248-253)- Uses the custom value scanner for pro-rating config with proper error handling (lines 254-259)
The type assertions and error handling follow the established patterns in the generated code.
367-372: Complete string representation including new fields.The
String()method properly includes both new fields in the string representation, maintaining consistency with the rest of the struct fields.openmeter/ent/db/plan.go (5)
14-15: LGTM! New import dependencies properly added.The imports for
productcatalogandisodatepackages are correctly added to support the new fields.
45-48: LGTM! New fields properly integrated into the Plan struct.The
BillingCadenceandProRatingConfigfields are correctly defined with appropriate types and JSON tags. The comments clearly describe their purpose.
110-115: LGTM! Scanning logic correctly updated for new fields.The
scanValuesmethod properly handles the new fields:
BillingCadenceis correctly added to the string field caseProRatingConfiguses the appropriate custom scanner from the value scanner registry
207-218: LGTM! Assignment logic properly handles new field types.The
assignValuesmethod correctly:
- Handles
BillingCadencewith proper type assertion and conversion toisodate.String- Uses the custom
FromValuemethod forProRatingConfigwith proper error handling
321-326: LGTM! String representation includes new fields.The
String()method is properly updated to include both new fields in the debug output.e2e/productcatalog_test.go (6)
210-216: LGTM! Plan creation properly includes new billing configuration.The
planCreatestruct correctly includes theBillingCadencefield set to "P1M", which establishes a monthly billing cycle as the default.
234-244: LGTM! Custom plan input includes comprehensive pro-rating configuration.The
customPlanInputproperly includes:
BillingCadenceinherited from the plan creationProRatingConfigwith appropriate default values (mode: "prorate_prices", enabled: true)This ensures custom plans have consistent billing and pro-rating behavior.
388-395: LGTM! Plan update includes billing cadence configuration.The plan update request properly includes the
BillingCadencefield, ensuring billing configuration is maintained during plan modifications.
439-442: LGTM! Comprehensive assertions verify subscription billing configuration.The test properly verifies that subscriptions inherit the billing configuration from plans:
BillingCadenceis correctly set to "P1M"ProRatingConfighas the expected mode and enabled stateThis ensures the new fields are properly propagated from plans to subscriptions.
497-500: LGTM! Plan-based subscription creation maintains billing configuration.The assertions confirm that subscriptions created from published plans inherit the correct billing cadence and pro-rating configuration, demonstrating proper end-to-end functionality.
639-644: LGTM! New plan version creation includes billing configuration.The new plan version creation properly includes the
BillingCadencefield, ensuring version consistency for billing configuration.openmeter/ent/schema/productcatalog.go (3)
5-5: LGTM! Appropriate import added for database dialect support.The
dialectimport is correctly added to support JSONB schema type specification for PostgreSQL.
36-51: LGTM! Well-designed schema fields for billing configuration.The new fields are properly defined:
billing_cadence:
- Uses
isodate.Stringtype for ISO8601 duration format- Clear comment describing its purpose as default billing cadence
pro_rating_config:
- Uses custom Go type with appropriate value scanner
- JSONB storage in PostgreSQL for structured data
- Sensible default enabling pro-rating with "prorate_prices" mode
- Clear comment describing its purpose
The schema design follows best practices and provides good defaults.
200-200: LGTM! Value scanner properly registered for ProRatingConfig.The
ProRatingConfigValueScanneris correctly added to support JSON serialization/deserialization of theProRatingConfigstruct, following the same pattern as other complex types in the schema.openmeter/productcatalog/pro_rating.go (5)
1-8: LGTM! Clean package structure and appropriate imports.The package declaration and imports are well-organized, bringing in necessary dependencies for error handling (
errors,fmt) and validation utilities (modelspackage).
10-20: LGTM! Well-designed enumeration type with extensibility.The
ProRatingModetype is properly implemented:
- Clear constant definition for "prorate_prices" mode
Values()method provides enumeration support for validation and documentation- Design allows for easy addition of new pro-rating modes in the future
22-29: LGTM! Clean struct definition with appropriate JSON tags.The
ProRatingConfigstruct is well-designed:
- Clear field names and purposes with good documentation
- Proper JSON tags for API serialization
- Simple boolean flag for enabling/disabling functionality
- Mode field using the custom enum type for type safety
31-43: LGTM! Robust validation with proper error handling.The
Validate()method is well-implemented:
- Uses switch statement for mode validation (extensible for new modes)
- Proper error collection and aggregation
- Uses the models package for consistent error handling across the system
- Returns descriptive error messages for invalid modes
45-48: LGTM! Simple and correct equality comparison.The
Equal()method correctly compares both fields of the struct, providing a reliable way to check configuration equivalence.openmeter/productcatalog/subscription/http/mapping.go (3)
196-200: LGTM!The billing cadence and pro-rating configuration mapping is implemented correctly. The conversion from domain types to API types follows the established patterns in the codebase.
456-460: LGTM!The plan creation request mapping properly handles the new fields with appropriate error handling for billing cadence parsing and sensible defaults for pro-rating configuration.
Also applies to: 476-480
497-510: LGTM!The helper function provides sensible defaults for pro-rating configuration and handles the nil case appropriately. The default of enabling pro-rating with "prorate_prices" mode is reasonable.
openmeter/ent/db/subscription/subscription.go (3)
10-11: LGTM!The generated field constants and imports are consistent with Ent conventions and properly support the new billing cadence and pro-rating configuration fields.
Also applies to: 45-48
114-115: LGTM!The generated code correctly adds the new fields to the Columns slice and includes proper ValueScanner support for the custom ProRatingConfig type.
Also applies to: 147-154
225-233: LGTM!The generated ordering functions for the new fields follow the established patterns and are implemented correctly.
openmeter/subscription/subscriptionspec.go (3)
35-39: LGTM!The new fields are well-documented and use appropriate types. The BillingCadence field using
isodate.Periodand ProRatingConfig using the domain type are good design choices.
65-77: LGTM!The method correctly includes the new billing cadence and pro-rating configuration fields in the entity input, maintaining consistency with the existing field mapping pattern.
314-347: LGTM!The billing cadence alignment validation is well-implemented. It properly checks all billable items across phases and uses the
DivisibleBymethod to ensure cadences are compatible. The logic correctly handles equal cadences and validates divisibility in both directions.tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql (3)
1-48: LGTM!The migration logic for plans is well-designed. The CTE approach to derive billing cadence from the last phase's rate cards is correct, and the strategy of populating all records (including soft-deleted) before making columns NOT NULL is sound. The default values are sensible.
50-102: LGTM!The subscriptions migration correctly implements the same pattern as plans but with appropriate complexity for the subscription data model. The use of
ROW_NUMBER()to identify the last phase and derive billing cadence from subscription items is well-implemented. The consistent default values maintain alignment between plans and subscriptions.
7-32: Consider performance and data validation.The migration logic is sound, but the complex CTEs could be slow on large datasets. Consider:
- Performance: Test the migration on a dataset similar to production size
- Data validation: Verify that the assumption about last phases (duration IS NULL for plans, latest active_from for subscriptions) holds for your data
- Rollback: Ensure the corresponding down migration is properly tested
The migration strategy is well-designed, but it would be beneficial to validate the assumptions about data structure.
Also applies to: 56-86
openmeter/ent/db/plan/plan.go (5)
10-11: LGTM: Imports correctly added for new field types.The imports for
fieldandproductcatalogpackages are appropriately added to support the new ProRatingConfig ValueScanner and type definitions.
41-44: LGTM: Field constants properly defined.The new field constants
FieldBillingCadenceandFieldProRatingConfigfollow the existing naming convention and map to the correct database column names.
94-95: LGTM: New fields correctly added to Columns slice.The new fields are properly included in the Columns slice, maintaining the correct order and ensuring they're recognized as valid columns.
129-136: LGTM: ProRatingConfig handling properly implemented.The default value function and ValueScanner for ProRatingConfig are correctly defined. The absence of similar handling for BillingCadence suggests it's a simple type that doesn't require complex scanning logic.
197-205: LGTM: Ordering functions consistently implemented.Both new fields have corresponding ordering functions that follow the established pattern and naming convention used by other fields in the entity.
openmeter/ent/db/migrate/schema.go (3)
1730-1732: Schema changes align with PR objectives.The addition of
billing_cadenceandpro_rating_configcolumns to the plans table is consistent with the PR's goal. Using JSONB forpro_rating_configis appropriate for storing structured configuration data in PostgreSQL.
2000-2001: Consistent schema changes for subscriptions table.The columns match those added to the plans table, maintaining consistency across the schema.
2013-2013: Foreign key and index updates are correct.The column index adjustments (shifting by 2) correctly account for the two new columns added to the subscriptions table.
Also applies to: 2019-2019, 2043-2043
pkg/isodate/date.go (4)
57-60: LGTM!Simple wrapper method that delegates to the underlying Period implementation.
62-91: Well-implemented period-to-days conversion.The method correctly handles all period components using decimal arithmetic to avoid precision issues. Note that it uses a uniform
daysInMonthvalue for all months, which is a reasonable approximation for billing calculations.
113-117: LGTM!Standard comparison implementation using subtraction and sign checking.
119-144: Robust divisibility check implementation.Excellent approach testing divisibility across different month lengths (28-31 days) to ensure the method works correctly regardless of the actual calendar month. The use of decimal arithmetic ensures precise calculations without floating-point errors.
openmeter/ent/db/subscription_update.go (1)
20-21: LGTM! Auto-generated code follows ent patterns correctly.The implementation properly integrates the new
billing_cadenceandpro_rating_configfields into the subscription update builders:
- Setter methods with nillable variants are correctly implemented
- Validation calls
Validate()onpro_rating_configas expected- SQL save logic uses the value scanner for proper JSONB conversion
Also applies to: 163-189, 358-362, 414-423, 734-760, 942-946, 1015-1024
openmeter/ent/db/plan_update.go (1)
19-20: LGTM! Consistent implementation with subscription update builders.The auto-generated code properly handles the new
billing_cadenceandpro_rating_configfields in the plan update builders, maintaining consistency with the subscription implementation.Also applies to: 143-169, 367-371, 420-429, 704-730, 941-945, 1011-1020
openmeter/productcatalog/plan/httpdriver/mapping.go (4)
33-34: LGTM! Clean field additions for the new billing features.The new fields are properly integrated into the API response mapping, following the existing pattern in the codebase.
105-105: Well-implemented field parsing with proper error handling.The ProRatingConfig assignment and BillingCadence parsing are correctly implemented with appropriate error handling and descriptive error messages.
Also applies to: 126-130
147-153: Simple and correct domain-to-API mapping helper.The helper function provides a clean conversion between domain and API representations.
205-205: Excellent handling of optional fields in update requests.The implementation correctly treats both fields as optional updates, with proper nil/empty checks and consistent error handling.
Also applies to: 214-222
tools/migrate/entitlement_test.go (3)
410-420: Good improvement for handling nullable database fields.Using
sql.NullStringproperly handles cases where the annotations field is NULL in the database, preventing scan errors and providing more accurate test assertions.
438-456: Consistent nullable field handling pattern.The defensive check using
annotationsJSON.Validbefore unmarshalling prevents potential errors and makes the test more robust.
845-860: Maintains consistent nullable field handling throughout the test file.The pattern is consistently applied, improving test reliability when dealing with optional database fields.
api/openapi.yaml (5)
19595-19614: Additive Change:billingCadenceandproRatingConfigfields are read-only
The addition of read-only fields is backward-compatible and enhances the subscription schema.
20264-20286: Additive Change: IntroducedProRatingConfigandProRatingModeschemas
The new component schemas are defined correctly and enhance pro-rating configurability.
21382-21386: Verify Service Implementation forbillingCadencein Subscription
SincebillingCadenceis now required in the subscription response, confirm that all subscription endpoints populate this field.
21857-21861: Verify Service Implementation forbillingCadencein Subscription Expanded
Ensure the new required field is present in all variants of the subscription response or consider making it optional.
21938-21959: Additive Change: Subscription schemas now includebillingCadenceandproRatingConfig
Good to support proration controls—these read-only additions are backward-compatible.tools/migrate/productcatalog_test.go (5)
15-61: LGTM! Well-structured migration test.The test properly validates the migration behavior for plan phase deletion, with appropriate setup and verification steps.
63-178: LGTM! Comprehensive ISO format migration test.The test effectively validates the entitlement migration logic, including the conversion of legacy formats to ISO8601 and the soft-deletion of non-standard intervals.
180-732: Excellent comprehensive migration test with detailed documentation!The test thoroughly validates the billing cadence migration logic with great attention to detail, including PostgreSQL interval casting verification and proper handling of edge cases.
734-1274: LGTM! Consistent and thorough subscription migration test.The test maintains the same high quality as the plan migration test, with comprehensive coverage of subscription-specific scenarios.
793-797: Good use of parameterized test data generation.The use of
fmt.Sprintffor generating unique customer IDs and emails is appropriate for test data creation.api/client/javascript/src/client/schemas.ts (5)
3507-3524: Excellent documentation and type safety for new billing fields.The addition of
billingCadenceandproRatingConfigfields is well-documented with clear examples and proper typing. The ISO8601 duration format specification and examples (P1M, P3M, P1Y) provide good guidance for API consumers.
7954-7971: Consistent field additions across plan schemas.The
billingCadenceandproRatingConfigfields are consistently added across CustomPlanInput, Plan, PlanCreate, and PlanReplaceUpdate schemas with identical documentation and typing. This maintains API consistency.Also applies to: 8166-8183, 8289-8306
9297-9314: Appropriate readonly designation for subscription schemas.The
billingCadenceandproRatingConfigfields are correctly marked asreadonlyin Subscription and SubscriptionExpanded schemas, which makes sense since these are typically inherited from the associated plan and shouldn't be directly modifiable on the subscription.Also applies to: 9666-9683
10915-10916: Proper type exports for new schemas.The new
ProRatingConfigandProRatingModetypes are correctly exported, maintaining consistency with other schema exports in the file.
8433-8452:✅ Verification successful
Consider the single enum value design choice.
The
ProRatingModeenum currently only contains'prorate_prices'. While this might be intentional for future extensibility, it's worth verifying this design choice.
🏁 Script executed:
#!/bin/bash # Description: Check if there are plans for additional pro-rating modes in the codebase # Expected: Find references to other potential pro-rating modes or TODO comments echo "Searching for pro-rating mode references and TODOs..." rg -i "prorate|pro.rating" --type go -A 3 -B 3 echo -e "\nSearching for TODO/FIXME comments related to billing or pro-rating..." rg -i "todo|fixme|hack" --type go | rg -i "billing|prorate|cadence"Length of output: 68368
Confirm single-value ProRatingMode is intentional
All searches across Go and JS/TS code found only
ProRatingModeProratePricesand no TODO/FIXME comments suggesting additional modes. Defining a single-value enum here is a valid extensibility pattern and requires no change.openmeter/ent/db/plan/where.go (3)
11-11: LGTM: Import added correctly.The
isodatepackage import is properly added to support the new billing cadence predicate functions.
119-123: LGTM: Shorthand predicate function follows established pattern.The
BillingCadenceshorthand function is implemented consistently with other field predicates in this file, properly convertingisodate.Stringtostringfor SQL operations.
660-742: LGTM: Comprehensive predicate functions correctly generated.The complete set of predicate functions for the billing_cadence field is properly implemented:
- All standard SQL operations (EQ, NEQ, In, NotIn, GT, GTE, LT, LTE)
- String operations (Contains, HasPrefix, HasSuffix, EqualFold, ContainsFold)
- Consistent type conversion from
isodate.Stringtostring- Follows the same patterns as other custom type fields in this file
The generated code is consistent and complete.
openmeter/ent/db/subscription/where.go (3)
12-12: LGTM: Import added correctly.The
isodatepackage import is properly added to support the new billing cadence predicate functions.
131-135: LGTM: Shorthand predicate function follows established pattern.The
BillingCadenceshorthand function is implemented consistently with other field predicates likeCurrency, properly convertingisodate.Stringtostringfor SQL operations.
806-888: LGTM: Comprehensive predicate functions correctly generated.The complete set of predicate functions for the billing_cadence field is properly implemented:
- All standard SQL operations (EQ, NEQ, In, NotIn, GT, GTE, LT, LTE)
- String operations (Contains, HasPrefix, HasSuffix, EqualFold, ContainsFold)
- Consistent type conversion from
isodate.Stringtostring- Follows the same patterns as other custom type fields like
currencyx.Code- Mirrors the implementation in the plan entity for consistency
The generated code is consistent and complete across both plan and subscription entities.
api/client/javascript/src/zod/index.ts (3)
10076-10081: Constants definition looks good.The default values for
proRatingConfigare properly defined and will be reused consistently across schemas.
11064-11080: Pro-rating configuration structure is well-designed.The
proRatingConfigobject structure withenabledboolean andmodeenum is clean and includes appropriate defaults. The single enum value'prorate_prices'is likely intentional for the initial implementation.Also applies to: 12091-12107, 14855-14873, 17342-17360
11118-11123: Default constants are consistently defined.The default values for
proRatingConfigare consistently defined across all schema contexts, ensuring uniform behavior.Also applies to: 13718-13725, 16237-16244
api/openapi.cloud.yaml (7)
13156-13156: Include billingCadence as required on Plan schema
AddingbillingCadenceto the required list ensures that every Plan object returned by the API explicitly declares its billing cadence.
19127-19127: Include billingCadence as required on Subscription schema
Ensures that API responses for Subscription always include the billing cadence field.
19419-19419: Include billingCadence as required on CustomPlanInput schema
Enforces that new or updated custom plans supply a billing cadence.
19607-19607: Include billingCadence as required on PlanReplaceUpdate schema
Ensures replacement payloads also mandate the billing cadence field.
19840-19863: Add ProRatingConfig and ProRatingMode component definitions
The newProRatingConfigandProRatingModeschemas correctly capture the pro-rating structure and allowed modes. Well-aligned with domain requirements.
20851-20851: Include billingCadence as required on SubscriptionExpanded schema
This change ensures the expanded subscription object always includes its billing cadence.
21326-21326: Include billingCadence as required on SubscriptionAddon schema
Ensures subscription addon schemas also require billing cadence.openmeter/ent/db/plan_create.go (4)
1-1: Note: This is a generated file - ensure schema changes are made in the sourceThis file is generated by ent. Any manual modifications will be lost upon regeneration. Ensure that all changes are made in the appropriate schema files and migration scripts.
145-163: LGTM! Setter methods follow established patternsThe new setter methods for
billing_cadenceandpro_rating_configare properly implemented following the existing patterns in the file.
359-369: Validation properly implemented for new fieldsBoth
billing_cadenceandpro_rating_configare correctly validated as required fields, with additional custom validation forProRatingConfigthrough itsValidate()method.
457-464:✅ Verification successful
Good error handling for ProRatingConfig value conversion
The value conversion and error handling for
ProRatingConfigis properly implemented. The use ofValueScannersuggests JSON serialization for database storage.Verify that the
ProRatingConfigValueScanner properly handles all edge cases:
🏁 Script executed:
#!/bin/bash # Check the ValueScanner implementation for ProRatingConfig ast-grep --pattern 'ProRatingConfigValueScanner' # Check for ProRatingConfig JSON tags and validation rg -A 10 'type ProRatingConfig struct'Length of output: 2274
🏁 Script executed:
#!/bin/bash # Locate the JSONStringValueScanner definition in the entutils package rg -R 'JSONStringValueScanner' -n . # Print surrounding lines to inspect its implementation rg -R -A 20 -B 5 'JSONStringValueScanner' -n .Length of output: 892
🏁 Script executed:
#!/bin/bash # Locate the JSONStringValueScanner definition rg 'JSONStringValueScanner' -n . # Print surrounding lines to inspect its implementation rg -n -A 20 -B 5 'JSONStringValueScanner' .Length of output: 8243
ValueScanner Implementation Verified
The
JSONStringValueScannerused forProRatingConfigmarshals viajson.Marshaland unmarshals viajson.Unmarshal, correctly handling:
- Nil or invalid
sql.NullString(returns zero value with no error)- Invalid JSON (error propagated)
- Struct and pointer types as expected
No further issues found—error handling and edge cases are properly covered.
openmeter/ent/db/subscription_create.go (1)
1-1: Note: This is a generated file - changes mirror plan_create.goThis file follows the same implementation patterns as
plan_create.gofor the newbilling_cadenceandpro_rating_configfields.openmeter/ent/db/mutation.go (16)
38672-38673: LGTM! Field declarations follow ent conventions.The new optional fields are properly declared as pointers, maintaining consistency with the existing mutation pattern.
39250-39320: LGTM! Complete and consistent implementation of billing_cadence and pro_rating_config methods.The setter, getter, old value retrieval, and reset methods are properly implemented following ent framework conventions. Error handling and nil checking are appropriate.
39616-39616: LGTM! Field capacity correctly updated.The capacity increase from 13 to 15 accounts for the two new fields being added.
39650-39655: LGTM! New fields properly added to Fields() method.The billing_cadence and pro_rating_config fields are correctly appended to the fields slice with proper nil checking.
39692-39695: LGTM! Field access correctly implemented.The new fields are properly handled in the Field() method switch statement.
39731-39734: LGTM! Old field access correctly implemented.The new fields are properly handled in the OldField() method switch statement.
39825-39838: LGTM! Type-safe field setting with proper error handling.The SetField() method correctly handles type checking for both new fields with appropriate error messages.
39983-39988: LGTM! Field reset properly implemented.The ResetField() method correctly handles resetting the new fields.
43814-43815: LGTM! Subscription mutation fields match plan mutation.The field declarations are consistent between Plan and Subscription mutations.
44472-44542: LGTM! Subscription mutation methods mirror plan implementation.The implementation is consistent with the PlanMutation methods, maintaining the same patterns and error handling.
44794-44794: LGTM! Subscription field capacity correctly updated.The capacity is properly increased to accommodate the new fields.
44834-44839: LGTM! Subscription Fields() method properly updated.The new fields are correctly added to the subscription fields slice.
44874-44877: LGTM! Subscription Field() method correctly handles new fields.The field access is properly implemented for both new fields.
44913-44916: LGTM! Subscription OldField() method correctly handles new fields.The old field access is properly implemented for both new fields.
45017-45030: LGTM! Subscription SetField() method with proper type checking.The type checking and field setting are correctly implemented with appropriate error handling.
45152-45157: LGTM! Subscription ResetField() method correctly handles new fields.The field reset functionality is properly implemented for both new fields.
tothandras
force-pushed
the
feat/plan-billing-cadence
branch
from
ad30221 to
53061d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (15)
openmeter/subscription/subscription.go (1)
30-31: LGTM! New billing fields properly defined.The new fields are correctly added to the
Subscriptionstruct:
BillingCadenceuses the appropriateisodate.Periodtype for ISO8601 duration representationProRatingConfiguses the expectedproductcatalog.ProRatingConfigtype- JSON tags follow the consistent snake_case naming convention
Note: The previous concern about these fields missing from the
AsEntityInputmethod has been addressed in PR #2922 as mentioned in the past review comments.openmeter/ent/db/runtime.go (2)
1168-1172: LGTM on pro_rating_config initialization, but billing_cadence still missingThe
pro_rating_configfield initialization is correctly implemented following the established pattern. However, thebilling_cadencefield initialization is still missing as noted in previous reviews.The previous review comment about missing
billing_cadenceinitialization for the plan entity is still valid and applicable.
1344-1348: LGTM on pro_rating_config initialization, but billing_cadence still missingThe
pro_rating_configfield initialization is correctly implemented following the established pattern. However, thebilling_cadencefield initialization is still missing as noted in previous reviews.The previous review comment about missing
billing_cadenceinitialization for the subscription entity is still valid and applicable.openmeter/productcatalog/plan.go (1)
106-159: Address the previous review comments about error handling and magic numbers.The issues raised in the previous review are still valid:
- The
Comparemethod returns magic numbers (-1, 0, 1) that should be documented or use named constants- When
DivisibleByreturns an error, the code continues to check the boolean result which may be invalidopenmeter/ent/db/subscription_create.go (1)
372-382: Add subscription–plan billing cadence compatibility validationopenmeter/ent/db/plan_create.go (1)
303-306: Missing default value for billing_cadence fieldThe
defaults()method sets a default forpro_rating_configbut not forbilling_cadence, even thoughbilling_cadenceis validated as a required field. This will cause creation to fail if callers don't explicitly set the billing cadence.Migration tests expect a "P1M" default, so this needs to be addressed by implementing
plan.DefaultBillingCadence()as suggested in the previous review.api/openapi.yaml (4)
13063-13067: Duplicate Issue:billingCadenceadded as required
This field is marked as required in the Plan replacement schema, constituting a breaking change for clients. Consider making it optional or supplying a default value to maintain backward compatibility.
19520-19524: Duplicate Issue: RequiredbillingCadencein plan version
AddingbillingCadenceto the required properties will break existing workflows—ensure default values or make it optional.
19812-19816: Duplicate Issue:billingCadencein required plan identifier
AddingbillingCadenceas a required property in the plan identifier schema can break client integrations. Consider making this field optional or providing a default.
20000-20004: Duplicate Issue: RequiredbillingCadencein plan replacement input
MakingbillingCadencerequired in this object is a breaking client change. Provide a default or make it optional to avoid disruptions.api/client/javascript/src/zod/index.ts (5)
10188-10192: Missing ISO8601 duration validation.This is the same issue flagged in previous reviews - the
billingCadencefield lacks validation for the ISO8601 duration format despite the description specifying it.
11230-11234: Multiple issues: missing validation and should be optional.This field has the same issues previously flagged:
- Missing ISO8601 duration validation
- Should be optional for update operations since users might not want to modify this field in every update
12091-12107: Should make billingCadence optional for update operations.As mentioned in previous reviews, for update schemas,
billingCadenceshould likely be optional since users might not want to modify this field in every update operation.
13937-13941: Missing ISO8601 duration validation.This is the same validation issue flagged in previous reviews for the
billingCadencefield in subscription custom plan schemas.
16424-16428: Missing ISO8601 duration validation.This is the same validation issue previously identified for the
billingCadencefield in change subscription custom plan schemas.
🧹 Nitpick comments (9)
openmeter/ent/schema/productcatalog.go (1)
39-51: Review the default pro-rating configuration.The
pro_rating_configfield is well-designed using JSONB storage for PostgreSQL and a custom value scanner. However, the default configuration enables pro-rating by default, which may not be appropriate for all use cases.Consider whether pro-rating should be disabled by default to avoid unexpected billing behavior for existing plans.
DefaultFunc(func() productcatalog.ProRatingConfig { return productcatalog.ProRatingConfig{ Mode: productcatalog.ProRatingModeProratePrices, - Enabled: true, + Enabled: false, } }).pkg/isodate/date.go (1)
118-144: Consider using more descriptive variable names.The single-letter variable names
landsreduce code readability. Consider using more descriptive names.-func (larger Period) DivisibleBy(smaller Period) (bool, error) { - l := larger.Simplify(true) - s := smaller.Simplify(true) +func (larger Period) DivisibleBy(smaller Period) (bool, error) { + largerSimplified := larger.Simplify(true) + smallerSimplified := smaller.Simplify(true) - if l.IsZero() || s.IsZero() || l.Compare(s) < 0 { + if largerSimplified.IsZero() || smallerSimplified.IsZero() || largerSimplified.Compare(smallerSimplified) < 0 { return false, nil } testDaysInMonth := []int{28, 29, 30, 31} for _, daysInMonth := range testDaysInMonth { // replace months with days - ldays, err := l.InDays(daysInMonth) + largerDays, err := largerSimplified.InDays(daysInMonth) if err != nil { return false, err } - sdays, err := s.InDays(daysInMonth) + smallerDays, err := smallerSimplified.InDays(daysInMonth) if err != nil { return false, err } - if _, r, err := ldays.QuoRem(sdays); err != nil || !r.IsZero() { + if _, remainder, err := largerDays.QuoRem(smallerDays); err != nil || !remainder.IsZero() { return false, err } } return true, nil }openmeter/productcatalog/plan/httpdriver/mapping.go (1)
147-169: Consider documenting the default ProRatingConfig behavior.The helper functions are well-implemented. The
asProRatingConfigfunction provides sensible defaults when the configuration is not provided. Consider adding a comment to document this default behavior for future maintainers.// asProRatingConfig converts API ProRatingConfig to domain ProRatingConfig +// When nil is provided, it returns a default configuration with proration enabled and ProratePrices mode func asProRatingConfig(p *api.ProRatingConfig) productcatalog.ProRatingConfig {api/client/javascript/src/client/schemas.ts (1)
8433-8452: Consider extensibility of the ProRatingMode enum.The
ProRatingModeenum currently only contains'prorate_prices'. While this may be sufficient for the current implementation, consider if additional pro-rating modes might be needed in the future (e.g.,'prorate_usage','no_proration').The current implementation with a single enum value is acceptable for initial release, but the enum structure allows for easy extension when additional pro-rating modes are required.
api/openapi.yaml (5)
19598-19606: Extract Common Property Definitions to Reuse
The inline definitions forbillingCadenceandproRatingConfigare duplicated across multiple schemas. Consider extracting them into reusable components (e.g.,BillingCadence) and referencing via$refto reduce duplication and improve maintainability.
19853-19869: Extract Common Property Definitions to Reuse
The definitions forbillingCadenceandproRatingConfigare repeated here. Refactor to reference shared schema components to improve consistency and reduce duplication.
20028-20044: Extract Common Property Definitions to Reuse
This block duplicates thebillingCadenceandproRatingConfigdefinitions. Consider centralizing these undercomponents/schemasand using$reffor a DRY schema design.
21469-21479: Extract Common Property Definitions to Reuse
Inline definitions ofbillingCadenceandproRatingConfigappear again. Centralize these incomponents/schemasfor consistency and maintainability.
21941-21951: Extract Common Property Definitions to Reuse
Repeated inline definitions forbillingCadenceandproRatingConfig—refactor to use$refto shared schemas and reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (66)
api/client/javascript/src/client/schemas.ts(8 hunks)api/client/javascript/src/zod/index.ts(12 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/package.json(1 hunks)api/spec/src/productcatalog/plan.tsp(2 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)e2e/productcatalog_test.go(6 hunks)openmeter/billing/worker/subscription/sync_test.go(7 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(16 hunks)openmeter/ent/db/plan.go(5 hunks)openmeter/ent/db/plan/plan.go(5 hunks)openmeter/ent/db/plan/where.go(3 hunks)openmeter/ent/db/plan_create.go(11 hunks)openmeter/ent/db/plan_update.go(7 hunks)openmeter/ent/db/predicate/predicate.go(2 hunks)openmeter/ent/db/runtime.go(2 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(5 hunks)openmeter/ent/db/subscription/where.go(3 hunks)openmeter/ent/db/subscription_create.go(12 hunks)openmeter/ent/db/subscription_update.go(7 hunks)openmeter/ent/schema/productcatalog.go(3 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/adapter/feature_test.go(1 hunks)openmeter/productcatalog/addon/adapter/mapping.go(2 hunks)openmeter/productcatalog/errors.go(1 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/plan/adapter/mapping.go(2 hunks)openmeter/productcatalog/plan/adapter/plan.go(2 hunks)openmeter/productcatalog/plan/httpdriver/mapping.go(6 hunks)openmeter/productcatalog/plan/serializer_test.go(1 hunks)openmeter/productcatalog/plan/service.go(3 hunks)openmeter/productcatalog/plan/service/plan.go(1 hunks)openmeter/productcatalog/plan/service/service_test.go(1 hunks)openmeter/productcatalog/plan_test.go(13 hunks)openmeter/productcatalog/planaddon_test.go(3 hunks)openmeter/productcatalog/pro_rating.go(1 hunks)openmeter/productcatalog/subscription/http/mapping.go(4 hunks)openmeter/productcatalog/subscription/plan.go(1 hunks)openmeter/productcatalog/subscription/service/plan.go(1 hunks)openmeter/productcatalog/testutils/plan.go(2 hunks)openmeter/subscription/addon/diff/restore_test.go(2 hunks)openmeter/subscription/aligment_test.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(1 hunks)openmeter/subscription/repository.go(1 hunks)openmeter/subscription/subscription.go(2 hunks)openmeter/subscription/subscriptionspec.go(3 hunks)openmeter/subscription/subscriptionview.go(1 hunks)openmeter/subscription/testutils/plan.go(1 hunks)openmeter/subscription/workflow/service/addon_test.go(2 hunks)openmeter/subscription/workflow/service/subscription.go(1 hunks)openmeter/subscription/workflow/service/subscription_test.go(2 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)test/billing/subscription_test.go(2 hunks)test/customer/customer.go(3 hunks)test/subscription/scenario_editaligned_test.go(3 hunks)test/subscription/scenario_editcancel_test.go(2 hunks)tools/migrate/entitlement_test.go(5 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql(1 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql(1 hunks)tools/migrate/productcatalog_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (26)
openmeter/subscription/workflow/service/addon_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/repo/subscriptionrepo.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)api/api.gen.go (1)
ProRatingConfig(5852-5858)api/client/go/client.gen.go (1)
ProRatingConfig(5382-5388)
openmeter/productcatalog/adapter/feature_test.go (4)
api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)
test/billing/subscription_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/subscription/service/plan.go (2)
pkg/models/errors.go (1)
ErrorWithFieldPrefix(340-346)pkg/models/fieldselector.go (2)
NewFieldSelectors(229-231)NewFieldSelector(177-179)
openmeter/productcatalog/errors.go (2)
pkg/models/errors.go (1)
ErrorCode(294-294)pkg/models/validationissue.go (3)
NewValidationIssue(219-230)WithFieldString(196-202)WithWarningSeverity(214-216)
openmeter/productcatalog/testutils/plan.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/adapter/mapping.go (2)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/subscription/workflow/service/subscription_test.go (2)
pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/ent/db/plan.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/plan.go (14)
FieldID(18-18)FieldNamespace(20-20)FieldName(30-30)FieldDescription(32-32)FieldKey(34-34)FieldCurrency(40-40)FieldBillingCadence(42-42)FieldCreatedAt(24-24)FieldUpdatedAt(26-26)FieldDeletedAt(28-28)FieldEffectiveFrom(46-46)FieldEffectiveTo(48-48)FieldProRatingConfig(44-44)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (11)
FieldID(18-18)FieldNamespace(20-20)FieldName(36-36)FieldDescription(38-38)FieldCurrency(44-44)FieldBillingCadence(46-46)FieldCreatedAt(22-22)FieldUpdatedAt(24-24)FieldDeletedAt(26-26)FieldProRatingConfig(48-48)ValueScanner(152-154)
openmeter/subscription/subscription.go (1)
pkg/isodate/date.go (1)
Period(47-49)
openmeter/subscription/addon/diff/restore_test.go (7)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)
openmeter/productcatalog/plan/adapter/plan.go (3)
openmeter/ent/db/plan/where.go (2)
BillingCadence(120-123)BillablesMustAlign(105-107)openmeter/ent/db/planratecard/where.go (1)
BillingCadence(111-114)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
e2e/productcatalog_test.go (4)
api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/planaddon_test.go (3)
openmeter/ent/db/plan/where.go (5)
Key(100-102)Version(110-112)Name(90-92)Currency(115-117)BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/pro_rating.go (5)
api/client/javascript/src/client/schemas.ts (2)
ProRatingMode(10916-10916)ProRatingConfig(10915-10915)api/api.gen.go (3)
ProRatingMode(5861-5861)ProRatingModeProratePrices(700-700)ProRatingConfig(5852-5858)api/client/go/client.gen.go (3)
ProRatingMode(5391-5391)ProRatingModeProratePrices(637-637)ProRatingConfig(5382-5388)pkg/models/validator.go (1)
Validate(16-26)pkg/models/errors.go (1)
NewNillableGenericValidationError(129-135)
openmeter/ent/db/plan/plan.go (4)
openmeter/ent/db/subscription/subscription.go (5)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)ByBillingCadence(226-228)OrderOption(158-158)ByProRatingConfig(231-233)openmeter/ent/db/planratecard/planratecard.go (3)
FieldBillingCadence(45-45)ByBillingCadence(206-208)OrderOption(143-143)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/ent/schema/subscription.go (4)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)openmeter/ent/schema/productcatalog.go (1)
ProRatingConfigValueScanner(200-200)
openmeter/subscription/workflow/service/subscription.go (1)
pkg/models/errors.go (1)
NewGenericValidationError(138-140)
openmeter/ent/db/subscription/subscription.go (7)
openmeter/ent/db/plan/plan.go (5)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)ByBillingCadence(198-200)OrderOption(140-140)ByProRatingConfig(203-205)openmeter/ent/db/subscriptionitem/subscriptionitem.go (3)
FieldBillingCadence(58-58)ByBillingCadence(257-259)OrderOption(159-159)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
OrderOption(285-285)openmeter/ent/db/entitlement/entitlement.go (1)
OrderOption(195-195)openmeter/ent/db/subscriptionphase/subscriptionphase.go (1)
OrderOption(113-113)
openmeter/ent/schema/productcatalog.go (4)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/plan/plan.go (1)
ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)
openmeter/ent/db/runtime.go (5)
openmeter/ent/db/plan/plan.go (2)
DefaultProRatingConfig(130-130)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (2)
DefaultProRatingConfig(148-148)ValueScanner(152-154)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/addon/addon.go (1)
ValueScanner(128-130)
api/client/javascript/src/client/schemas.ts (3)
api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingMode(5861-5861)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingMode(14-14)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingMode(5391-5391)
openmeter/ent/db/migrate/schema.go (2)
openmeter/ent/db/plan/plan.go (1)
Columns(81-98)openmeter/ent/db/subscription/subscription.go (1)
Columns(99-116)
openmeter/ent/db/subscription_update.go (6)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/ent.go (2)
ValidationError(255-258)Value(65-65)openmeter/ent/db/subscription/where.go (2)
Name(106-108)BillingCadence(132-135)openmeter/ent/db/plan/plan.go (3)
FieldBillingCadence(42-42)ValueScanner(134-136)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (3)
FieldBillingCadence(46-46)ValueScanner(152-154)FieldProRatingConfig(48-48)
openmeter/ent/db/subscription_create.go (7)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/subscription/where.go (2)
BillingCadence(132-135)Name(106-108)openmeter/ent/db/ent.go (2)
ValidationError(255-258)Value(65-65)openmeter/ent/db/subscription.go (2)
Subscription(22-60)Subscription(129-148)openmeter/ent/schema/subscription.go (5)
Subscription(18-20)Subscription(22-31)Subscription(33-57)Subscription(59-64)Subscription(66-77)openmeter/subscription/subscription.go (1)
Subscription(13-32)
🔇 Additional comments (165)
api/spec/package.json (1)
29-29: LGTM! Package manager version update looks good.The pnpm version update from 10.7.0 to 10.11.0 with the corresponding integrity hash is properly applied. This helps maintain current tooling for the project.
openmeter/productcatalog/adapter/feature_test.go (1)
322-326: LGTM! Proper initialization of new billing fields in test setup.The test correctly initializes the new
BillingCadenceandProRatingConfigfields:
BillingCadenceset to "P1M" (monthly billing) aligns with typical billing cyclesProRatingConfigproperly configured with enabled pro-rating using theProRatingModeProratePricesmodeThis ensures the test environment is consistent with the extended data model.
openmeter/productcatalog/plan/serializer_test.go (1)
37-41: LGTM! Proper test coverage for new billing fields in serialization.The test correctly includes the new
BillingCadenceandProRatingConfigfields in thePlanMetainitialization:
isodate.MustParse(t, "P1M")is appropriate for test scenarios and will provide clear failure messages if the period is invalid- The pro-rating configuration aligns with the expected defaults and structure
This ensures that the JSON serialization/deserialization of these new fields works correctly.
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql (1)
1-4: LGTM! Correct rollback migration for billing cadence and pro-rating columns.The down migration properly removes the
pro_rating_configandbilling_cadencecolumns from both theplansandsubscriptionstables, effectively reversing the schema changes introduced in the corresponding up migration.openmeter/subscription/subscription.go (1)
9-9: LGTM! Necessary import for isodate support.The import of
pkg/isodateis required for the newBillingCadencefield usingisodate.Periodtype.openmeter/productcatalog/subscription/plan.go (1)
74-77: LGTM: Clean implementation of new billing configuration fields.The addition of
BillingCadenceandProRatingConfigfields to the returned struct follows the existing pattern and correctly propagates these fields from the plan to the subscription input.openmeter/subscription/aligment_test.go (1)
36-40: LGTM: Proper test setup for new billing configuration.The test correctly adds the new
BillingCadenceandProRatingConfigfields using appropriate test values. The use ofisodate.MustParse(t, "P1M")for one-month billing cadence and enabling pro-rating withProRatingModeProratePricesaligns with the system-wide changes.openmeter/subscription/workflow/service/subscription_test.go (2)
28-28: LGTM: Required import addition.The import of
isodatepackage is necessary for theMustParsefunction used in the test setup.
867-871: LGTM: Consistent test configuration for billing features.The addition of
BillingCadenceandProRatingConfigto the second example plan maintains consistency with other test files and properly configures the new billing features for testing.test/subscription/scenario_editcancel_test.go (2)
24-24: LGTM: Necessary import for test functionality.The addition of the
isodateimport is required for the billing cadence parsing functionality used in the test.
61-65: LGTM: Comprehensive test setup with new billing configuration.The test properly includes the new billing cadence and pro-rating configuration fields, ensuring that the scenario testing covers the enhanced billing model. The values are consistent with other test files and use appropriate defaults.
openmeter/subscription/workflow/service/addon_test.go (2)
27-27: LGTM: Import addition is appropriate.The import of
isodatepackage is correctly added to support the new billing cadence functionality used in the test.
571-575: LGTM: Billing cadence and pro-rating configuration added correctly.The plan metadata has been properly enhanced with:
BillingCadenceset to monthly period ("P1M") using the appropriate parsing functionProRatingConfigwith enabled pro-rating andProRatingModeProratePricesmodeThis aligns with the broader system changes and follows the pattern established in other files.
test/billing/subscription_test.go (2)
115-123: LGTM: Paid plan metadata correctly enhanced.The
PlanMetafor the paid plan has been properly updated with:
BillingCadenceset to daily period ("P1D")ProRatingConfigwith enabled pro-rating and correct modeThe daily billing cadence is appropriate for this test scenario.
166-174: LGTM: Free plan metadata correctly enhanced.The
PlanMetafor the free plan has been consistently updated with the same billing cadence and pro-rating configuration as the paid plan, which is appropriate for this test focusing on profile changes rather than billing differences.openmeter/subscription/repo/mapping.go (2)
31-34: LGTM: Billing cadence parsing with proper error handling.The billing cadence parsing logic is implemented correctly:
- Uses the appropriate
Parse()method from the database field- Includes proper error handling with a descriptive error message
- Follows the established error handling pattern in this function
56-62: LGTM: Field assignments are correct.The subscription struct is properly updated with the new fields:
BillingCadenceuses the parsed value from the parsing logic aboveProRatingConfigis directly assigned from the database entityThe field assignments are consistent with the function's mapping pattern.
openmeter/productcatalog/planaddon_test.go (3)
46-54: LGTM: "Valid" test case plan metadata enhanced correctly.The plan metadata in the "valid" test case has been properly updated with:
BillingCadenceset to monthly period ("P1M") which aligns with the rate cards in the testProRatingConfigwith appropriate settingsThis ensures the test data is complete and valid according to the new schema.
214-222: LGTM: "Invalid" test case plan metadata enhanced correctly.The plan metadata in the "invalid" test case has been consistently updated with the same billing cadence and pro-rating configuration as other test cases, ensuring uniform test data structure.
409-417: LGTM: "Incompatible" test case plan metadata enhanced correctly.The plan metadata in the "incompatible" test case has been properly updated, maintaining consistency with other test cases while preserving the test's intended validation behavior.
openmeter/productcatalog/errors.go (1)
462-470: LGTM! Error code addition follows established patterns.The new error code and validation issue for plan billing cadence compatibility follows the existing conventions in the file, with appropriate warning severity and clear messaging.
test/customer/customer.go (3)
22-22: Import addition supports new functionality.The
isodateimport is correctly added to support the new billing cadence field parsing in the test plan metadata.
226-232: Test plan metadata properly updated with new required fields.The addition of
BillingCadenceandProRatingConfigfields ensures the test plan metadata is complete with the newly introduced billing configuration. The values used (P1M billing cadence and enabled pro-rating) are appropriate for testing scenarios.
560-567: Consistent test plan configuration across test scenarios.The billing cadence and pro-rating configuration values match those used in the other test case, ensuring consistency in test data setup.
openmeter/subscription/testutils/plan.go (1)
59-67: Test plan builder properly updated with new billing configuration fields.The addition of
BillingCadenceandProRatingConfigto the test plan builder ensures consistent test plan metadata across all tests using this utility. The values used are consistent with other test files in the codebase.openmeter/subscription/addon/diff/restore_test.go (2)
23-23: Import addition enables billing cadence support in tests.The
isodateimport is properly added to support the new billing cadence parsing functionality used in the test plan metadata.
354-358: Test plan metadata updated for addon restore functionality.The billing cadence and pro-rating configuration fields are properly added to ensure the addon restore tests work correctly with plans that include these new billing configuration settings. The values are consistent with other test files.
openmeter/subscription/repository.go (1)
29-34: LGTM! Well-structured addition of billing cadence and pro-rating configuration.The new fields are properly typed with appropriate documentation and JSON tags. The
isodate.Periodtype for billing cadence andproductcatalog.ProRatingConfigfor pro-rating configuration align with the domain model requirements.test/subscription/scenario_editaligned_test.go (2)
23-23: LGTM! Proper import addition for ISO date handling.
57-64: LGTM! Comprehensive test coverage for new billing configuration fields.The test updates properly set both
BillingCadenceandProRatingConfigfields with reasonable values:
- Monthly billing cadence (
P1M) is a common real-world scenario- Pro-rating enabled with
ProRatingModeProratePricesprovides good test coverage- Both aligned and unaligned subscription scenarios are consistently updated
Also applies to: 238-245
openmeter/productcatalog/plan/service/plan.go (1)
754-762: LGTM! Proper metadata preservation in plan versioning.The
NextPlanmethod correctly preserves the new billing configuration fields (Alignment,BillingCadence,ProRatingConfig) when creating new plan versions. This ensures consistency across plan versions and follows the established pattern for other metadata fields.openmeter/subscription/repo/subscriptionrepo.go (1)
106-107: LGTM! Proper persistence of billing configuration fields.The subscription creation correctly persists the new fields:
BillingCadenceis properly converted to ISO string format for database storageProRatingConfigis set directly, appropriate for the JSONB database fieldThe implementation follows the established pattern for other subscription metadata fields.
openmeter/productcatalog/plan/service/service_test.go (1)
28-30: LGTM! Clean refactoring to ISO 8601 duration format.The refactoring from package-level approximate day-based durations to local variables using precise ISO 8601 duration strings aligns well with the project-wide standardization of billing cadence representation. The use of
isodate.MustParse(t, "P1M")is appropriate for test code.openmeter/productcatalog/plan/adapter/mapping.go (2)
17-20: Proper billing cadence parsing with good error handling.The implementation correctly parses the billing cadence from the database row using the
Parse()method and includes descriptive error handling that shows the invalid value.
39-40: Correct field assignment for new billing features.The assignment of both
BillingCadenceandProRatingConfigto thePlanMetastruct is consistent with the broader PR changes introducing these fields.openmeter/productcatalog/subscription/service/plan.go (1)
63-64: Enhanced error handling with better context.The use of
models.ErrorWithFieldPrefixwith field selectors provides better context for validation errors by prefixing them with the "plan" field. This is an improvement over generic error wrapping and aligns with enhanced error reporting throughout the codebase.openmeter/subscription/subscriptionview.go (1)
241-246: Proper propagation of billing cadence and pro-rating configuration.The expansion of
CreateSubscriptionPlanInputto includeBillingCadenceandProRatingConfigfields from the subscription input ensures these new fields flow correctly through the subscription creation and management pipeline. This change aligns well with the comprehensive billing cadence and pro-rating support introduced in this PR.openmeter/productcatalog/plan_test.go (3)
13-13: LGTM: Appropriate import addition for ISO date parsing.The import of the
isodatepackage is necessary to support the newBillingCadencefield initialization usingisodate.MustParse.
33-37: LGTM: Consistent test data enhancement with new billing fields.The systematic addition of
BillingCadenceandProRatingConfigfields across all test cases ensures that the test data aligns with the extended plan metadata model. The chosen values are appropriate:
BillingCadence: isodate.MustParse(t, "P1M")- Monthly billing cadenceProRatingConfigwithEnabled: trueandProRatingModeProratePrices- Sensible defaults for testingUsing
isodate.MustParseis appropriate in test contexts as it will fail fast if the duration string is invalid.Also applies to: 50-54, 67-71, 84-88, 101-105, 118-122, 135-139, 152-156, 169-173
197-203: LGTM: Consistent field additions in alignment enforcement tests.The addition of the required fields in the
TestAlignmentEnforcementtest cases maintains consistency with the overall changes and ensures these validation scenarios include the new billing cadence and pro-rating configuration data.Also applies to: 252-258, 307-313
openmeter/productcatalog/testutils/plan.go (2)
11-11: LGTM: Required import for ISO date parsing support.The import addition enables the use of
isodate.MustParsefor initializing theBillingCadencefield.
29-33: LGTM: Enhanced test utility with new billing configuration fields.The addition of
BillingCadenceandProRatingConfigfields to theNewTestPlanfunction ensures that all test plans created via this utility include realistic billing cadence and pro-rating configuration data. The values are consistent with those used in the main test file, promoting consistency across the test suite.openmeter/ent/db/predicate/predicate.go (2)
215-224: LGTM: Generated predicate follows established pattern.The
PlanOrErrfunction follows the same error-aware predicate pattern as other*OrErrfunctions in this generated file, providing consistent error handling behavior for Plan predicates.
249-258: LGTM: Generated predicate follows established pattern.The
SubscriptionOrErrfunction maintains consistency with the established error-aware predicate pattern, enabling proper error handling for Subscription predicates.openmeter/subscription/workflow/service/subscription.go (1)
144-151: LGTM: Proper billing cadence alignment validation.The addition of billing cadence alignment validation in the
EditRunningmethod is well-placed and follows good practices:
- Timing: Validation occurs after applying customizations but before committing the transaction
- Error handling: Uses
models.NewGenericValidationErrorconsistently with the codebase pattern- Business logic: Ensures billing cadence consistency within subscriptions, which is critical for proper billing operations
The validation prevents inconsistent billing cadences that could lead to billing issues downstream.
api/spec/src/productcatalog/prorating.tsp (1)
1-31: LGTM! Well-structured type definitions.The pro-rating configuration types are cleanly defined with:
- Clear documentation and appropriate decorators
- Sensible default values (enabled: true, mode: proratePrices)
- Extensible union type for future pro-rating modes
- Consistent naming conventions
openmeter/productcatalog/plan/adapter/plan.go (2)
183-184: LGTM! Proper integration of new fields in plan creation.The new
BillingCadenceandProRatingConfigfields are correctly added to the plan creation flow using appropriate methods:
BillingCadenceusesISOString()for proper ISO8601 formattingProRatingConfigis set directly as expected
428-430: LGTM! Consistent field handling in plan updates.The nullable setters for the new fields allow for optional updates while maintaining consistency with existing update patterns in the codebase.
pkg/isodate/date_test.go (1)
81-217: LGTM! Comprehensive test coverage for period divisibility.Excellent test implementation that covers:
- Compatible period combinations (1Y/1M, 1Y/2M, etc.)
- Incompatible combinations (1M/3D, 1M/1W, etc.)
- Edge cases like same periods
- Proper error handling verification
The test structure is clean with descriptive names and follows Go testing best practices. This provides solid coverage for the
DivisibleBymethod used in billing cadence alignment validation.e2e/productcatalog_test.go (4)
215-215: LGTM! Proper integration of billing cadence in plan creation.Adding
BillingCadence: "P1M"ensures the new field is tested in the main plan creation flow.
240-243: LGTM! Well-configured pro-rating settings.The
ProRatingConfigis properly set with:
Mode: "prorate_prices"using the correct string valueEnabled: trueas expected default behavior
439-441: LGTM! Comprehensive verification of new fields.The assertions properly verify that both custom subscriptions and plan-based subscriptions inherit the billing cadence and pro-rating configuration correctly:
BillingCadencematches expected "P1M" valueProRatingConfig.Modeuses the proper API constantProRatingModeProratePricesProRatingConfig.Enabledis verified as trueThis ensures end-to-end functionality works as expected.
Also applies to: 497-499
390-390: LGTM! Consistent field usage across test scenarios.The billing cadence field is properly included in both plan update and plan version creation scenarios, ensuring comprehensive test coverage.
Also applies to: 643-643
openmeter/ent/db/plan.go (6)
14-15: LGTM! Imports are correctly added.The new imports for
productcatalogandisodatepackages are necessary for the new field types and are properly added.
45-48: LGTM! Field definitions are well-structured.The new fields are properly defined with appropriate types and JSON tags:
BillingCadenceusesisodate.Stringwhich aligns with the ISO8601 duration formatProRatingConfiguses the domain-specificproductcatalog.ProRatingConfigtype- Comments clearly describe the purpose of each field
110-110: LGTM! Billing cadence correctly added to string field scanning.Adding
plan.FieldBillingCadenceto the string fields case is correct sinceisodate.Stringshould be scanned as a string from the database.
114-115: LGTM! Pro-rating config uses appropriate custom scanner.Using
plan.ValueScanner.ProRatingConfig.ScanValue()is the correct approach for JSONB fields that require custom type conversion.
207-218: LGTM! Assignment logic correctly handles both field types.The assignment logic properly handles:
BillingCadence: Converts fromsql.NullStringtoisodate.Stringwith null checkingProRatingConfig: Uses the customValueScanner.FromValue()method with proper error handling
321-326: LGTM! String representation properly includes new fields.Both new fields are correctly added to the string representation using
fmt.Sprintf, maintaining consistency with the existing field formatting.api/spec/src/productcatalog/plan.tsp (3)
4-4: LGTM! Import correctly added for pro-rating types.The import of
"./prorating.tsp"is necessary for theProRatingConfigandProRatingModetypes used in the plan model.
83-92: LGTM! Billing cadence field is well-defined.The
billingCadencefield definition is excellent:
- Uses appropriate
durationtype for ISO8601 durations- Proper visibility for Read, Create, and Update operations
- Correct
@encode(DurationKnownEncoding.ISO8601)annotation- Clear documentation with practical examples (monthly, quarterly, annually)
- Valid example value using
duration.fromISO("P1M")
94-102: LGTM! Pro-rating configuration field is properly structured.The
proRatingConfigfield definition is well-implemented:
- Optional field (
ProRatingConfig?) which is appropriate for default configurations- Sensible default value with pro-rating enabled and prorate prices mode
- Proper visibility settings for Read, Create, and Update operations
- Clear documentation explaining its purpose
api/spec/src/productcatalog/subscription.tsp (2)
95-104: LGTM! Billing cadence field appropriately defined for subscriptions.The
billingCadencefield is well-designed for subscriptions:
- Required field (not optional) which is appropriate since active subscriptions must have a billing cadence
- Read-only visibility makes sense as this would typically be inherited from the plan
- Proper ISO8601 encoding and clear documentation
- Consistent example with the plan model
106-114: LGTM! Pro-rating configuration consistently defined.The
proRatingConfigfield definition is well-structured:
- Optional field with default value matching the plan model defaults
- Read-only visibility is appropriate for subscription context
- Default value enables pro-rating with prorate prices mode, which is sensible
- Clear documentation explaining its purpose
openmeter/productcatalog/addon/adapter/mapping.go (2)
187-191: LGTM! Proper parsing and error handling for billing cadence.The implementation correctly parses the billing cadence from the database row using the
Parse()method and provides clear error messaging with context when parsing fails.
207-209: LGTM! Correct assignment of new fields to PlanMeta.The assignment of
BillingCadenceandProRatingConfigfields to thePlanMetastruct is consistent with the existing pattern and correctly uses the parsed billing cadence value.openmeter/ent/schema/subscription.go (2)
40-42: Verify if billing_cadence should have validation constraints.The
billing_cadencefield uses the appropriateisodate.Stringtype for ISO8601 durations. However, consider whether this field should have validation constraints likeNotEmpty()orRequired()to ensure data integrity.Please verify if billing cadence should be a required field or can be optional/null for subscriptions.
43-55: LGTM! Well-structured pro-rating configuration field.The implementation is correct with appropriate JSONB storage for PostgreSQL and a sensible default configuration. The default function sets
Enabled: trueandMode: ProRatingModeProratePrices, which appears to be a reasonable default for new subscriptions.openmeter/productcatalog/plan/service.go (3)
13-13: LGTM! Appropriate import addition.The
isodatepackage import is necessary for the newBillingCadencefield type.
155-159: LGTM! Well-structured optional fields for plan updates.The new fields are correctly defined as pointers to indicate they're optional for update operations. The types (
isodate.Periodandproductcatalog.ProRatingConfig) are appropriate, and the JSON tags follow the existing naming convention.
199-205: LGTM! Correct equality checks for new fields.The equality comparisons are properly implemented:
BillingCadenceuses string comparison via.String()methodProRatingConfiguses the.Equal()method for proper comparisonThe null checks ensure safe dereferencing of the pointer fields.
openmeter/billing/worker/subscription/sync_test.go (7)
157-165: LGTM! Consistent test data update for billing cadence and pro-rating.The addition of
BillingCadenceandProRatingConfigfields to thePlanMetastruct follows the expected pattern with valid ISO 8601 duration ("P1M") and appropriate pro-rating configuration settings.
574-582: LGTM! Consistent test data enhancement.The test data correctly includes the new billing cadence and pro-rating configuration fields with the same values as other test cases, maintaining consistency across the test suite.
1391-1395: LGTM! Proper test data alignment with schema changes.The
TestDefactoZeroPricestest case now includes the required billing cadence and pro-rating configuration fields, ensuring the test remains valid with the updated schema.
1467-1471: LGTM! Billing cadence updated appropriately for aligned subscription test.The test uses "P2W" (2 weeks) billing cadence which is appropriate for testing aligned subscription invoicing scenarios, and maintains consistent pro-rating configuration.
1690-1694: LGTM! Test data updated for subscription cancellation scenarios.The billing cadence and pro-rating configuration are correctly added to support testing subscription cancellation with the new schema fields.
1859-1863: LGTM! Progressive billing test enhanced with new fields.The test data for progressive billing cancellation scenarios now includes the necessary billing cadence and pro-rating configuration, maintaining test coverage for the updated functionality.
3806-3814: LGTM! Helper method updated to support new schema.The
createSubscriptionFromPlanPhaseshelper method now includes the required billing cadence and pro-rating configuration fields, ensuring all tests using this helper will have consistent test data setup.openmeter/ent/db/plan/plan.go (5)
10-11: LGTM: Imports added correctly for new field types.The imports for
fieldandproductcatalogpackages are necessary to support the new field definitions and types used in the Plan entity.
41-44: LGTM: Field constants follow established naming convention.The new field constants
FieldBillingCadenceandFieldProRatingConfigfollow the same naming pattern as existing field constants and correctly map to the database column names.
94-95: LGTM: New fields properly added to Columns slice.The new fields are correctly included in the
Columnsslice, maintaining proper ordering and ensuring they will be included in SQL operations.
197-205: LGTM: Ordering functions follow established pattern.The
ByBillingCadenceandByProRatingConfigordering functions correctly follow the same pattern as other ordering functions in the file, usingsql.OrderByFieldand returning the appropriate function type.
129-136: LGTM: Default function and value scanner properly configured.The
DefaultProRatingConfigfunction andValueScannerstruct withProRatingConfigfield are correctly defined and follow the patterns established for complex field types in the codebase.openmeter/ent/schema/productcatalog.go (3)
5-5: LGTM: Import added for schema type mapping.The
dialectimport is necessary to support theSchemaTypemapping for PostgreSQL JSONB storage of thepro_rating_configfield.
36-38: LGTM: Billing cadence field well-defined.The
billing_cadencefield correctly usesisodate.Stringtype which is appropriate for ISO8601 duration strings. The descriptive comment clearly explains the field's purpose.
200-200: LGTM: Value scanner correctly defined.The
ProRatingConfigValueScannerfollows the same pattern as other JSON value scanners in the file and uses the appropriate generic type forproductcatalog.ProRatingConfig.openmeter/productcatalog/pro_rating.go (4)
10-20: LGTM: ProRatingMode type and Values method well-implemented.The
ProRatingModetype with the constantProRatingModeProratePricesand theValues()method are correctly implemented. The method provides a way to enumerate valid values, which is useful for validation and API documentation.
22-29: LGTM: ProRatingConfig struct well-designed.The
ProRatingConfigstruct has appropriate fields with clear JSON tags and documentation. The structure allows for flexible pro-rating configuration while maintaining type safety.
31-43: LGTM: Validation method correctly implemented.The
Validate()method properly validates theModefield against known values and uses the models package for error handling. The validation logic is extensible for future modes and follows established error handling patterns.
45-48: LGTM: Equal method correctly implemented.The
Equal()method properly compares bothEnabledandModefields, providing a reliable way to check configuration equality.openmeter/ent/db/subscription.go (5)
16-18: LGTM: Imports added correctly for new field types.The imports for
productcatalogandisodatepackages are necessary to support the new field types in the Subscription entity.
52-55: LGTM: New fields properly added to Subscription struct.The
BillingCadenceandProRatingConfigfields are correctly defined with appropriate types and JSON tags. The field comments provide clear documentation of their purpose.
137-142: LGTM: scanValues updated correctly for new fields.The
scanValuesmethod properly handles the new fields:BillingCadenceis treated as a nullable string (consistent with other string fields), andProRatingConfiguses the custom value scanner for complex type handling.
248-259: LGTM: assignValues updated correctly for new fields.The
assignValuesmethod properly handles both new fields with appropriate type conversion and error handling. TheBillingCadenceassignment convertssql.NullStringtoisodate.String, andProRatingConfiguses the value scanner'sFromValuemethod with proper error handling.
367-372: LGTM: String method updated to include new fields.The
String()method correctly includes both new fields in the string representation, maintaining consistency with the existing format and field ordering.openmeter/ent/db/subscription/subscription.go (5)
10-11: LGTM! Imports are properly added for the new functionality.The new imports are correctly added to support the field value scanner and productcatalog types needed for the ProRatingConfig field.
45-48: LGTM! Field constants follow established naming conventions.The new field constants for
billing_cadenceandpro_rating_configdatabase columns are properly defined and follow the same naming pattern as existing fields.
114-115: LGTM! Columns slice correctly updated.The new fields are properly added to the Columns slice, ensuring they are included in SQL operations and column validation.
147-154: LGTM! Default value and value scanner appropriately implemented.The implementation correctly:
- Adds a default value function for ProRatingConfig (complex type requiring defaults)
- Includes ProRatingConfig in the ValueScanner struct for custom serialization
- Appropriately omits BillingCadence from these sections since it's a nullable string type that doesn't require special handling
225-233: LGTM! Ordering functions properly implemented.The new ordering functions for both BillingCadence and ProRatingConfig follow the same pattern as existing ordering functions and will enable proper query ordering by these fields.
openmeter/ent/db/migrate/schema.go (4)
1730-1731: LGTM - Plan table schema correctly updatedThe new
billing_cadenceandpro_rating_configcolumns are properly defined for the plans table. The JSONB schema type forpro_rating_configis appropriate for storing configuration data.
2000-2001: LGTM - Subscription table schema correctly updatedThe new
billing_cadenceandpro_rating_configcolumns mirror the plans table structure, maintaining consistency across related entities.
1730-1730:✅ Verification successful
Confirm migration strategy for non-nullable fields
Both new fields are defined as non-nullable without default values. While the AI summary mentions migration scripts that handle existing data, it's important to ensure the migration strategy properly populates these fields for existing records.
Please confirm that the migration scripts properly handle the non-nullable constraint by checking the migration files:
Also applies to: 2000-2000
🏁 Script executed:
#!/bin/bash # Description: Verify that migration scripts handle the non-nullable billing_cadence and pro_rating_config fields # Check for migration files related to billing cadence and pro-rating fd -e sql -e go | grep -i "billing.*cadence\|pro.*rating" | head -5 # Look for migration logic that populates these fields rg -A 10 -B 5 "billing_cadence.*pro_rating_config" --type sql || echo "No SQL patterns found" rg -A 10 -B 5 "ALTER.*TABLE.*(plans|subscriptions)" --type sql || echo "No table alteration patterns found"Length of output: 14907
Migrations correctly populate and enforce non-nullable fields
I’ve confirmed in tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql that:
- Both billing_cadence and pro_rating_config are added as nullable columns.
- Existing rows are backfilled via UPDATE statements:
- billing_cadence from the last phase rate cards (and subscription items).
- pro_rating_config set to
{"mode": "prorate_prices", "enabled": true}where null.- Both columns are then altered to
SET NOT NULLon plans and subscriptions.No further action is needed here.
2013-2013:✅ Verification successful
Verify foreign key and index column references are correctly updated
The column indices for foreign key references and indexes have been properly adjusted to account for the two new columns inserted before the existing
customer_idandplan_idfields. The updates from indices [12,13] to [14,15] are mathematically correct.Let me verify that all column references are consistently updated throughout the schema:
Also applies to: 2019-2019, 2043-2043
🏁 Script executed:
#!/bin/bash # Description: Verify that all SubscriptionsColumns references use the correct indices after the schema change # Check for any remaining references to old column indices that should have been updated echo "Checking for potentially outdated SubscriptionsColumns references..." rg "SubscriptionsColumns\[1[2-3]\]" --type go # Also check the referenced files to ensure consistency echo "Verifying column definitions in related files..." rg "customer_id.*plan_id" openmeter/ent/db/subscription/subscription.go || echo "File not found, checking pattern matches" rg "FieldCustomerID.*FieldPlanID" openmeter/ent/db/subscription/subscription.go || echo "Field pattern not found"Length of output: 605
🏁 Script executed:
#!/bin/bash echo "Locating subscription.go files..." find . -type f -name subscription.go || echo "No subscription.go files found" echo "Listing all SubscriptionsColumns references with file paths and line numbers..." rg "SubscriptionsColumns\[" -nLength of output: 1637
Foreign key and index column references are correctly updated
All occurrences of
SubscriptionsColumnshave been updated to use indices 14 and 15. A search confirmed there are no remaining references to the old indices (12 or 13) in the migration schema:
- openmeter/ent/db/migrate/schema.go lines 2013, 2019 – use
[14]and[15]as intended- Other references either point to index 0 or 1 and are unchanged
No further changes are needed.
openmeter/subscription/subscriptionspec.go (3)
34-40: LGTM! Fields added correctly with proper documentation.The new
BillingCadenceandProRatingConfigfields are properly typed and documented, following Go conventions.
65-70: LGTM! Method correctly updated to include new fields.The
ToCreateSubscriptionEntityInputmethod properly maps the newBillingCadenceandProRatingConfigfields from the spec.
314-347:✅ Verification successful
Verify the new isodate.Period methods and consider method naming.
The billing cadence alignment logic is well-implemented, but please verify that the new
CompareandDivisibleBymethods onisodate.Periodbehave correctly for all edge cases.Additionally, consider if the method name
HasAlignedBillingCadencesaccurately reflects its purpose - it seems to check compatibility/divisibility rather than strict alignment.Let me verify that the new isodate.Period methods are implemented correctly:
🏁 Script executed:
#!/bin/bash # Description: Verify the Compare and DivisibleBy methods exist on isodate.Period # Expected: Find method implementations for Compare and DivisibleBy # Search for Compare method implementation ast-grep --pattern $'func ($$$) Compare($$$) $$$' # Search for DivisibleBy method implementation ast-grep --pattern $'func ($$$) DivisibleBy($$$) $$$' # Also search specifically in isodate package fd "isodate" --type f --extension go --exec cat {} \; | grep -A 10 -E "(Compare|DivisibleBy)"Length of output: 1621
Verified
CompareandDivisibleByimplementations inisodate.Period.I confirmed that both methods are defined and handle edge cases in
pkg/isodate/date.go(Compare at lines 113–116; DivisibleBy at lines 119–144).No further action is required to make the alignment logic safe—these methods exist and behave as expected.
• File:
pkg/isodate/date.go
–func (p Period) Compare(p2 Period) int
–func (larger Period) DivisibleBy(smaller Period) (bool, error)Consider renaming
HasAlignedBillingCadencesto more clearly convey that it checks compatibility/divisibility (for example,IsBillingCadenceCompatibleorBillingCadencesAreCompatible).openmeter/productcatalog/subscription/http/mapping.go (3)
196-200: LGTM! API mapping correctly implemented.The new fields are properly mapped to their API representations with appropriate type conversions.
476-479: LGTM! Billing cadence parsing with proper error handling.The parsing logic correctly converts the ISO duration string to
isodate.Periodwith appropriate error handling and descriptive error messages.
497-510: LGTM! Helper function provides sensible defaults.The
asProRatingConfigfunction correctly handles nil input with reasonable defaults (enabled pro-rating with prorate prices mode) and properly maps provided values.tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql (4)
56-86: Subscription migration logic is more robust but interval casting concern remains.The subscription migration logic using
ROW_NUMBER() OVER (PARTITION BY sp.subscription_id ORDER BY sp.active_from DESC)to find the last phase is more robust than the plans approach. However, the same PostgreSQL interval casting concern applies here.
34-44: LGTM! Sensible defaults for new required fields.The default values are appropriate:
P1M(monthly) is a reasonable default billing cadence- Pro-rating enabled with
prorate_pricesmode is a sensible default- Applying defaults to all records (including soft-deleted) correctly satisfies the NOT NULL constraints
Also applies to: 88-98
46-48: LGTM! Safe migration pattern for adding required fields.The migration correctly follows the safe pattern of adding nullable columns, populating data, then adding NOT NULL constraints to avoid constraint violations.
Also applies to: 100-102
7-32:✅ Verification successful
Verify PostgreSQL interval casting and last phase detection logic.
The migration logic for finding billing cadences has some assumptions that should be verified:
Interval casting: The
billing_cadence::intervalcasting assumes all stored values are valid PostgreSQL intervals. Invalid ISO duration strings could cause the migration to fail.Last phase detection: Using
duration IS NULLto identify the last phase assumes this is how terminal phases are marked. This should be confirmed against the actual data model.Ordering behavior: PostgreSQL interval ordering might not behave as expected for all ISO duration formats.
🏁 Script executed:
#!/bin/bash # Description: Verify the last phase detection logic and interval casting behavior # Expected: Confirm that NULL duration indicates last phase and interval casting works # Check how phases are structured and if NULL duration indicates last phase ast-grep --pattern $'type $_ struct { $$$ Duration $$$ $$$ }' # Look for phase-related logic that might explain the duration field rg -A 5 -B 5 "duration.*NULL|Duration.*nil" --type go # Check if there are any existing interval casting examples rg "::interval" --type sqlLength of output: 35751
No changes required—migration logic is sound.
Both key assumptions have been validated:
- Terminal phases are reliably marked by
duration IS NULL(enforced byCreateSubscriptionPhaseInputdocs,productcatalog.Planvalidation, and exercised in multiple tests).- Casting
billing_cadence::intervalsucceeds for all stored ISO-duration strings and produces the correct ordering (covered by the query intools/migrate/productcatalog_test.go).openmeter/ent/db/plan_update.go (3)
143-169: LGTM! Generated setter methods follow consistent patterns.The new setter methods for
BillingCadenceandProRatingConfigare properly generated and follow the same patterns as existing fields, including nillable variants for optional updates.
367-371: LGTM! Proper validation logic for ProRatingConfig.The validation correctly calls the domain object's
Validate()method and wraps errors appropriately inValidationErrorwith the correct field name.Also applies to: 941-945
420-429: LGTM! Proper SQL field handling with appropriate type conversions.The SQL save logic correctly handles both field types:
BillingCadenceas a direct string fieldProRatingConfigusing the value scanner for proper JSON serialization with appropriate error handlingAlso applies to: 1011-1020
pkg/isodate/date.go (2)
57-59: LGTM!The method correctly delegates to the underlying period's
Simplifymethod, maintaining the wrapper pattern consistency.
61-90: Well-implemented period to days conversion.The method correctly handles decimal arithmetic with proper error propagation, which is essential for accurate billing calculations.
tools/migrate/entitlement_test.go (1)
21-22: Good practice: Explicit struct field naming.The changes to explicitly name the
stopsfield in the runner struct initialization improve code clarity and maintainability.Also applies to: 138-139, 495-496
openmeter/ent/db/subscription_update.go (2)
163-190: Generated code correctly implements new fields.The generated code properly adds support for
billing_cadenceandpro_rating_configfields with:
- Appropriate setter methods with nullable variants
- Proper validation for
ProRatingConfigusing itsValidate()method- Correct database serialization using
ValueScannerAlso applies to: 358-362, 414-423
734-760: Generated code consistency maintained in UpdateOne builder.The
SubscriptionUpdateOnebuilder correctly mirrors the implementation fromSubscriptionUpdatefor the new fields.Also applies to: 942-946, 1015-1024
openmeter/productcatalog/plan.go (4)
12-12: LGTM!The import is necessary for the new
BillingCadencefield.
213-217: LGTM!The new fields are well-documented and properly typed with appropriate JSON tags.
243-249: LGTM!The validation logic correctly ensures that
BillingCadenceis not empty and delegatesProRatingConfigvalidation appropriately.
276-282: LGTM!The equality checks are correctly implemented for both new fields.
openmeter/productcatalog/plan/httpdriver/mapping.go (3)
29-34: LGTM!The field mappings are correctly implemented with appropriate type conversions.
105-105: LGTM!The field conversions are properly implemented with appropriate error handling for
BillingCadenceparsing.Also applies to: 126-129
205-205: LGTM!The update request handling correctly processes the new fields with appropriate conditional logic and error handling.
Also applies to: 214-221
api/client/javascript/src/client/schemas.ts (3)
3507-3524: LGTM: Well-structured addition of billing configuration to plan schema.The new fields are properly defined with appropriate types and comprehensive documentation. The
billingCadenceas a required field andproRatingConfigas optional aligns with expected plan configuration patterns.
9297-9314: Appropriate use of readonly modifiers for subscription fields.The
readonlymodifiers on subscription interfaces correctly indicate that these billing configuration values are inherited from the associated plan and should not be directly modified on the subscription itself.Also applies to: 9666-9683
10915-10916: Proper export of new types for client usage.The new
ProRatingConfigandProRatingModetypes are correctly exported, making them available for use in client applications that need to work with billing configuration objects.api/openapi.cloud.yaml (13)
13153-13157: AddbillingCadenceto required fields in plan input schemaThis ensures that any new or replaced plan payload must explicitly include a billing cadence.
13187-13203: DefinebillingCadenceandproRatingConfigin plan creation schemaThe
billingCadencefield is correctly typed as an ISO8601 duration, andproRatingConfigreferences the newProRatingConfigschema with a sensible default setup.
19124-19128: AddbillingCadenceto required fields in plan representationEnsures that the server’s plan response will always include the billing cadence property.
19199-19218: AddbillingCadenceandproRatingConfigproperties to plan output schemaBoth properties are defined correctly; the default for
proRatingConfigwill enable proration by default in the API responses.
19416-19419: IncludebillingCadencein required fields for plan update schemaKeeps replace/update operations aligned with creation by making billing cadence mandatory.
19457-19473: Define billing cadence and pro-rating config in plan update schemaReuses the same property definitions as in creation, leveraging the shared
ProRatingConfigreference.
19604-19608: EnforcebillingCadencein replacement plan definitionEnsures that the replace/update plan object continues to require billing cadence in its payload.
19632-19648: Add billing cadence and pro-rating config to plan alignment schemaExtends the alignment-related plan schema with the new billing frequency and proration settings.
19840-19862: Define newProRatingConfigandProRatingModeschemasThe base object and enum schema for pro-rating configuration are correctly specified, covering both the enable flag and the mode selection.
20848-20851: RequirebillingCadencein subscription creation schemaEnsures that any new subscription payload includes the billing cadence parameter.
20935-20953: AddbillingCadenceandproRatingConfigto subscription output schemaBoth fields are appropriately marked
readOnly: truein the response model, reflecting server-generated values.
21323-21326: IncludebillingCadencein required fields for expanded subscription schemaMaintains consistency by enforcing billing cadence in the expanded subscription output.
21407-21424: Define billing cadence and pro-rating config in subscription item schemaAdds the new properties with defaults and read-only flags for subscription item models.
openmeter/ent/db/subscription/where.go (1)
12-12: LGTM!The auto-generated predicate functions for the
billing_cadencefield follow the established patterns correctly, with proper type conversion fromisodate.Stringtostring.Also applies to: 131-135, 806-888
openmeter/ent/db/plan/where.go (1)
11-11: LGTM!The auto-generated predicate functions for the
billing_cadencefield in plans are consistent with the subscription implementation and follow the established patterns.Also applies to: 119-123, 660-742
openmeter/ent/db/subscription_create.go (1)
21-23: Well-implemented support for new fieldsThe auto-generated code correctly implements:
- Setter methods with proper nil handling for
ProRatingConfig- Default value initialization with sensible defaults (
Enabled: true,Mode: ProRatingModeProratePrices)- Proper ValueScanner usage for JSONB conversion
- Comprehensive upsert support for both fields
Also applies to: 176-194, 316-319, 469-480, 741-763, 975-1001, 1383-1409
openmeter/ent/db/mutation.go (6)
38672-38673: Field declarations look correct.The new fields follow the same naming convention and pointer pattern as existing fields in the mutation struct.
39250-39320: Generated methods follow consistent patterns.The setter, getter, old value retriever, and reset methods for both fields are implemented correctly and follow the same conventions as existing fields in the mutation type.
39616-39616: Field count and slice updates are correct.The Fields() method properly increments the capacity from 13 to 15 and includes the new fields in the returned slice when they are set.
Also applies to: 39650-39655
39692-39695: Dynamic field handling implementation is correct.The Field(), OldField(), SetField(), and ResetField() methods properly handle the new fields with correct type assertions and consistent error handling patterns.
Also applies to: 39731-39734, 39825-39838, 39983-39988
38814-38815: SubscriptionMutation changes mirror PlanMutation correctly.The field declarations and method implementations for SubscriptionMutation follow the same correct patterns as those implemented for PlanMutation.
Also applies to: 44472-44542
44794-44794: SubscriptionMutation dynamic field handling is consistent and correct.All the dynamic field handling methods (Fields, Field, OldField, SetField, ResetField) for SubscriptionMutation properly implement support for the new fields with consistent patterns and correct type handling.
Also applies to: 44834-44839, 44874-44877, 44913-44916, 45017-45030, 45152-45157
tools/migrate/productcatalog_test.go (2)
180-732: Well-structured and comprehensive migration test!The test thoroughly covers all scenarios for plan billing cadence and pro-rating config migration:
- Multiple phases with different rate cards
- Proper PostgreSQL interval casting verification
- Default fallback behavior
- Down migration cleanup
The detailed comments explaining each test scenario are particularly helpful.
734-1274: Excellent subscription migration test coverage!The test mirrors the plan migration test structure while properly handling subscription-specific requirements:
- Creates necessary customer records for foreign key constraints
- Tests billing cadence selection from subscription items in the last phase
- Verifies default behavior and down migration
The implementation is thorough and well-documented.
api/openapi.yaml (2)
13097-13106: Schema Addition Looks Good
ThebillingCadenceandproRatingConfigproperties are well-defined with appropriate types, formats, and descriptive fields.
20264-20280: New Schemas for Pro-Rating Configuration
TheProRatingConfigandProRatingModeschemas are correctly defined with required fields, default values, and clear descriptions.api/client/javascript/src/zod/index.ts (7)
10076-10081: LGTM! Well-structured default constants.The default constants for pro-rating configuration are well-defined and consistently structured. The naming follows the established pattern and the values are appropriate.
11064-11080: Well-structured pro-rating configuration schema.The
proRatingConfigobject is properly defined with clear descriptions and appropriate defaults. The enum constraint formodeand boolean type forenabledare correct.
11118-11123: LGTM! Consistent default constants for update operations.The default constants for the update plan body pro-rating configuration maintain consistency with the create operation constants.
13718-13725: LGTM! Consistent constants for subscription custom plan.The default constants follow the same pattern as the plan schemas and maintain consistency across the codebase.
14855-14873: Well-implemented pro-rating configuration for subscription.The pro-rating configuration schema for the subscription custom plan is consistent with the plan schemas and properly structured.
16237-16244: LGTM! Consistent constants for change subscription operation.The default constants maintain the established pattern and are consistent with other schemas in the file.
17342-17360: Consistent implementation for change subscription pro-rating config.The pro-rating configuration schema for the change subscription operation maintains consistency with other schemas and is properly implemented.
tothandras
force-pushed
the
feat/plan-billing-cadence
branch
from
53061d2 to
5483c81
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
openmeter/productcatalog/plan/service.go (1)
222-261: 🛠️ Refactor suggestionAdd validation for new fields.
The
Validate()method should include validation for the newBillingCadenceandProRatingConfigfields to ensure data integrity.Consider adding validation logic:
if i.Phases != nil { for _, phase := range *i.Phases { if err := phase.Validate(); err != nil { errs = append(errs, fmt.Errorf("invalid plan phase: %w", err)) } } } + + if i.BillingCadence != nil { + if err := i.BillingCadence.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid billing cadence: %w", err)) + } + } + + if i.ProRatingConfig != nil { + if err := i.ProRatingConfig.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid pro-rating config: %w", err)) + } + }
♻️ Duplicate comments (18)
openmeter/subscription/subscription.go (1)
30-31: LGTM! New fields are properly defined.The
BillingCadenceandProRatingConfigfields are correctly added to theSubscriptionstruct:
BillingCadenceusesisodate.Periodfor ISO8601 duration representationProRatingConfiguses the appropriateproductcatalog.ProRatingConfigtype- JSON tags follow consistent snake_case naming convention
Note: Based on past review comments, the missing fields in
AsEntityInputmethod have been addressed in PR #2922.openmeter/ent/db/runtime.go (2)
1168-1172: Missing initialization forbilling_cadenceonplan.The
planschema now includes both thebilling_cadenceandpro_rating_configfields, but only the latter is initialized here. You must register the default and scanner forbilling_cadenceas well to prevent runtime panics.Apply this diff before the
pro_rating_configblock:+ // planDescBillingCadence is the schema descriptor for billing_cadence field. + planDescBillingCadence := planFields[2].Descriptor() + // plan.DefaultBillingCadence holds the default value on creation for the billing_cadence field. + plan.DefaultBillingCadence = planDescBillingCadence.Default.(func() string) + // plan.ValueScanner.BillingCadence scans the billing_cadence field. + plan.ValueScanner.BillingCadence = planDescBillingCadence.ValueScanner.(field.TypeValueScanner[string])
1344-1348: Missing initialization forbilling_cadenceonsubscription.Similar to the
planentity, thesubscriptionschema initialization includespro_rating_configbut notbilling_cadence. You must register the default and scanner forbilling_cadencehere as well to avoid runtime panics.Apply this diff before the
pro_rating_configblock:+ // subscriptionDescBillingCadence is the schema descriptor for billing_cadence field. + subscriptionDescBillingCadence := subscriptionFields[5].Descriptor() + // subscription.DefaultBillingCadence holds the default value on creation for the billing_cadence field. + subscription.DefaultBillingCadence = subscriptionDescBillingCadence.Default.(func() string) + // subscription.ValueScanner.BillingCadence scans the billing_cadence field. + subscription.ValueScanner.BillingCadence = subscriptionDescBillingCadence.ValueScanner.(field.TypeValueScanner[string])pkg/isodate/date.go (1)
112-116: Handle the error from Subtract operation.The error from
p.Period.Subtract(p2.Period)is being ignored, which could hide potential issues during period comparison.openmeter/productcatalog/plan.go (1)
106-159: Improve error handling when DivisibleBy returns an error.When
DivisibleByreturns an error, the code continues to check the boolean result which may be invalid. This issue was already identified in a previous review.api/client/javascript/src/client/schemas.ts (1)
3507-3524: Confirm billingCadence breaking change impact is acceptable.This adds the required
billingCadencefield to what appears to be thePlanCreateschema. As noted in previous reviews, making this field required creates a breaking change for existing API consumers who call plan creation endpoints without providing this field.The JSDoc documentation is well-structured with appropriate examples and format specifications. The
proRatingConfigis correctly marked as optional with sensible defaults.api/openapi.yaml (6)
13063-13067: Breaking Change: RequiredbillingCadencein Plan input schema
MakingbillingCadencea required property here introduces a breaking change for clients that create or replace plans without specifying this field. Consider removing it from therequiredlist or providing a default fallback to preserve backward compatibility.
19520-19524: Breaking Change: RequiredbillingCadencein Plan Version schema
RequiringbillingCadencehere will break existing workflows for plan versions lacking this field. Either remove it from therequiredarray, mark it optional, or ensure a default value is injected via migration or server-side logic.
19815-19816: Breaking Change: RequiredbillingCadencein Plan Identifier schema
AddingbillingCadenceas a required field here can break clients that rely on plan identifiers without this property. Consider making it optional or providing a default in all API responses.
20003-20004: Breaking Change: MakingbillingCadencerequired in Plan Alignment schema
RequiringbillingCadencein this alignment object is a breaking change. Unless all clients can supply it, consider marking it optional or injecting a default alignment cadence.
21385-21386: Breaking Change: RequiredbillingCadencein Subscription schema
MarkingbillingCadenceas required in the Subscription output schema may break existing clients that don't expect this property. Consider making it optional or ensuring the server always populates and documents this field.
21860-21861: Breaking Change: RequiredbillingCadencein SubscriptionExpanded schema
MakingbillingCadencerequired here could break clients expecting a partial SubscriptionExpanded. Either remove it fromrequiredor guarantee default behavior via server responses.openmeter/ent/db/subscription_create.go (1)
372-382: Validation logic is comprehensive.The validation ensures both
billing_cadenceandpro_rating_configare present, with additional validation forpro_rating_configusing itsValidate()method. The existing past review comment about subscription-plan compatibility validation remains valid.openmeter/ent/db/plan_create.go (1)
303-306: Missing default for required billing_cadence field.The existing past review comment correctly identifies that
billing_cadencelacks a default value despite being a required field. This inconsistency should be addressed.Also applies to: 359-361
api/client/javascript/src/zod/index.ts (4)
10188-10192: Add ISO8601 duration validation and consider required vs optional for different operations.The billingCadence field lacks validation for the ISO8601 duration format despite the description specifying examples like "P1M", "P3M", "P1Y". This is a duplicate of a previous review comment that should be addressed.
Add validation for the ISO8601 duration format:
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' )
11230-11234: Make billingCadence optional for update operations and add validation.This is a duplicate of previous review comments. For update schemas, billingCadence should be optional since users might not want to modify this field in every update operation. Additionally, the field still lacks ISO8601 duration validation.
Apply both fixes:
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ) + .optional()
13718-13725: Add ISO8601 duration validation for subscription custom plan billingCadence.The billingCadence field in the createSubscriptionBody customPlan schema has the same validation issue as the plan schemas - missing ISO8601 duration format validation.
Add validation:
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' )Also applies to: 13937-13941
16237-16244: Add ISO8601 duration validation for change subscription custom plan billingCadence.The billingCadence field in the changeSubscriptionBody customPlan schema has the same validation issue. Additionally, since this is a change/update operation, consider making billingCadence optional.
Add validation and consider making it optional:
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ) + .optional()Also applies to: 16424-16428
🧹 Nitpick comments (8)
openmeter/productcatalog/plan_test.go (1)
33-37: LGTM! Consistent test data updates across all test cases.The uniform addition of
BillingCadenceandProRatingConfigfields ensures all existing tests continue to work with the new required schema. The consistent use of 1-month billing cadence and enabled pro-rating provides stable test foundations.Consider adding test cases with different billing cadences and pro-rating configurations in future iterations to ensure edge cases are covered.
Also applies to: 50-54, 67-71, 84-88, 101-105, 118-122, 135-139, 152-156, 169-173, 197-203, 252-258, 307-313
api/openapi.cloud.yaml (1)
13187-13204: Add ISO8601 duration validation pattern.
Consider enforcing valid ISO8601 durations onbillingCadenceby adding apatternregex. For example:billingCadence: type: string format: duration + pattern: '^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$' description: |- The default billing cadence for subscriptions using this plan. Defines how often customers are billed using ISO8601 duration format. Examples: "P1M" (monthly), "P3M" (quarterly), "P1Y" (annually).tools/migrate/productcatalog_test.go (2)
180-732: Excellent comprehensive migration test with thorough PostgreSQL interval validation.The test effectively validates:
- PostgreSQL's ability to cast ISO8601 durations to intervals
- Correct selection of shortest billing cadence from rate cards
- Proper fallback to default 'P1M' for plans without rate cards
- Correct pro-rating config structure
The detailed inline documentation is particularly helpful for understanding the test scenarios.
Consider extracting helper functions for repeated patterns like plan/phase/rate card creation to improve maintainability:
func createTestPlan(t *testing.T, db *sql.DB, id, key, name string) { _, err := db.Exec(` INSERT INTO plans (namespace, id, created_at, updated_at, key, version, name, currency) VALUES ('default', $1, NOW(), NOW(), $2, 1, $3, 'USD')`, id, key, name, ) require.NoError(t, err) }
734-1274: Well-structured subscription migration test with proper validation.The test mirrors the plan migration test structure and thoroughly validates:
- Subscription billing cadence selection from items
- Proper handling of multiple phases
- Default fallback behavior
- Pro-rating config consistency
Similar to the plan migration test, consider extracting helper functions for creating test subscriptions, phases, and items to reduce code duplication and improve readability.
api/openapi.yaml (4)
13097-13113: Consider DRYing repeated property definitions
ThebillingCadenceandproRatingConfigdefinitions here duplicate those in other plan schemas (e.g., version, identifier, alignment). To reduce maintenance overhead, consider extracting these into reusable components or YAML anchors and referencing them via$ref.
19598-19614: Reduce duplication of plan version properties
Similar to the base plan schema,billingCadenceandproRatingConfigare repeatedly defined here. Extract common definitions into reusable components or anchors to promote DRY and ensure consistency across schemas.
19853-19869: Consider centralizing shared schema properties
The definitions ofbillingCadenceandproRatingConfighere mirror other plan schemas. Refactoring these into shared components/anchors will simplify maintenance and reduce the risk of divergence.
20028-20044: DRY opportunity for Plan Alignment schema
billingCadenceandproRatingConfigdefinitions here are duplicates of those in plan and version schemas. Extract to shared components/anchors to keep schemas consistent and easier to update.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (66)
api/client/javascript/src/client/schemas.ts(8 hunks)api/client/javascript/src/zod/index.ts(12 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/package.json(1 hunks)api/spec/src/productcatalog/plan.tsp(2 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)e2e/productcatalog_test.go(6 hunks)openmeter/billing/worker/subscription/sync_test.go(7 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(16 hunks)openmeter/ent/db/plan.go(5 hunks)openmeter/ent/db/plan/plan.go(5 hunks)openmeter/ent/db/plan/where.go(3 hunks)openmeter/ent/db/plan_create.go(11 hunks)openmeter/ent/db/plan_update.go(7 hunks)openmeter/ent/db/predicate/predicate.go(2 hunks)openmeter/ent/db/runtime.go(2 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(5 hunks)openmeter/ent/db/subscription/where.go(3 hunks)openmeter/ent/db/subscription_create.go(12 hunks)openmeter/ent/db/subscription_update.go(7 hunks)openmeter/ent/schema/productcatalog.go(3 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/adapter/feature_test.go(1 hunks)openmeter/productcatalog/addon/adapter/mapping.go(2 hunks)openmeter/productcatalog/errors.go(1 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/plan/adapter/mapping.go(2 hunks)openmeter/productcatalog/plan/adapter/plan.go(2 hunks)openmeter/productcatalog/plan/httpdriver/mapping.go(6 hunks)openmeter/productcatalog/plan/serializer_test.go(1 hunks)openmeter/productcatalog/plan/service.go(3 hunks)openmeter/productcatalog/plan/service/plan.go(1 hunks)openmeter/productcatalog/plan/service/service_test.go(1 hunks)openmeter/productcatalog/plan_test.go(13 hunks)openmeter/productcatalog/planaddon_test.go(3 hunks)openmeter/productcatalog/pro_rating.go(1 hunks)openmeter/productcatalog/subscription/http/mapping.go(4 hunks)openmeter/productcatalog/subscription/plan.go(1 hunks)openmeter/productcatalog/subscription/service/plan.go(1 hunks)openmeter/productcatalog/testutils/plan.go(2 hunks)openmeter/subscription/addon/diff/restore_test.go(2 hunks)openmeter/subscription/aligment_test.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(1 hunks)openmeter/subscription/repository.go(1 hunks)openmeter/subscription/subscription.go(2 hunks)openmeter/subscription/subscriptionspec.go(3 hunks)openmeter/subscription/subscriptionview.go(1 hunks)openmeter/subscription/testutils/plan.go(1 hunks)openmeter/subscription/workflow/service/addon_test.go(2 hunks)openmeter/subscription/workflow/service/subscription.go(1 hunks)openmeter/subscription/workflow/service/subscription_test.go(2 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)test/billing/subscription_test.go(2 hunks)test/customer/customer.go(3 hunks)test/subscription/scenario_editaligned_test.go(3 hunks)test/subscription/scenario_editcancel_test.go(2 hunks)tools/migrate/entitlement_test.go(5 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql(1 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql(1 hunks)tools/migrate/productcatalog_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (32)
openmeter/productcatalog/plan/serializer_test.go (7)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)
openmeter/productcatalog/errors.go (1)
pkg/models/validationissue.go (3)
NewValidationIssue(219-230)WithFieldString(196-202)WithWarningSeverity(214-216)
openmeter/subscription/repo/subscriptionrepo.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)api/api.gen.go (1)
ProRatingConfig(5852-5858)api/client/go/client.gen.go (1)
ProRatingConfig(5382-5388)
openmeter/subscription/addon/diff/restore_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/service/service_test.go (1)
pkg/isodate/date.go (1)
MustParse(183-190)
openmeter/subscription/workflow/service/subscription.go (1)
pkg/models/errors.go (1)
NewGenericValidationError(138-140)
openmeter/productcatalog/testutils/plan.go (4)
openmeter/ent/db/plan/where.go (3)
Name(90-92)Currency(115-117)BillingCadence(120-123)api/api.gen.go (3)
Currency(1876-1888)ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/service/plan.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/productcatalog/alignment.go (1)
Alignment(3-6)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/subscription/workflow/service/addon_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/workflow/service/subscription_test.go (2)
pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/subscription/service/plan.go (2)
pkg/models/errors.go (1)
ErrorWithFieldPrefix(340-346)pkg/models/fieldselector.go (2)
NewFieldSelectors(229-231)NewFieldSelector(177-179)
openmeter/productcatalog/addon/adapter/mapping.go (1)
openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/subscription/repo/mapping.go (5)
openmeter/ent/db/subscription/where.go (5)
BillingCadence(132-135)Name(106-108)Description(111-113)CustomerID(121-123)Currency(126-129)openmeter/ent/db/subscription.go (2)
Subscription(22-60)Subscription(129-148)openmeter/subscription/subscription.go (1)
Subscription(13-32)openmeter/subscription/plan.go (1)
PlanRef(11-15)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
test/subscription/scenario_editaligned_test.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/adapter/plan.go (5)
openmeter/ent/db/plan/where.go (2)
BillingCadence(120-123)BillablesMustAlign(105-107)openmeter/ent/db/subscription/where.go (2)
BillingCadence(132-135)BillablesMustAlign(101-103)openmeter/ent/db/planratecard/where.go (1)
BillingCadence(111-114)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/subscription/subscription.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)pkg/isodate/date.go (1)
Period(47-49)
openmeter/productcatalog/plan/service.go (2)
pkg/isodate/date.go (1)
Period(47-49)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
test/billing/subscription_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/ent/db/subscription/subscription.go (4)
openmeter/ent/db/plan/plan.go (5)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)ByBillingCadence(198-200)OrderOption(140-140)ByProRatingConfig(203-205)openmeter/ent/db/subscriptionitem/subscriptionitem.go (3)
FieldBillingCadence(58-58)ByBillingCadence(257-259)OrderOption(159-159)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/ent/db/subscription.go (6)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/subscription/subscription.go (10)
FieldID(18-18)FieldNamespace(20-20)FieldName(36-36)FieldDescription(38-38)FieldPlanID(40-40)FieldCustomerID(42-42)FieldCurrency(44-44)FieldBillingCadence(46-46)FieldProRatingConfig(48-48)ValueScanner(152-154)
openmeter/productcatalog/adapter/feature_test.go (1)
openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/pro_rating.go (5)
api/client/javascript/src/client/schemas.ts (2)
ProRatingMode(10916-10916)ProRatingConfig(10915-10915)api/api.gen.go (3)
ProRatingMode(5861-5861)ProRatingModeProratePrices(700-700)ProRatingConfig(5852-5858)api/client/go/client.gen.go (3)
ProRatingMode(5391-5391)ProRatingModeProratePrices(637-637)ProRatingConfig(5382-5388)pkg/models/validator.go (1)
Validate(16-26)pkg/models/errors.go (1)
NewNillableGenericValidationError(129-135)
openmeter/subscription/aligment_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(183-190)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/ent/schema/subscription.go (4)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)openmeter/ent/schema/productcatalog.go (1)
ProRatingConfigValueScanner(200-200)
openmeter/ent/schema/productcatalog.go (4)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/plan/plan.go (1)
ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)
openmeter/productcatalog/plan/adapter/mapping.go (2)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/ent/db/subscription_update.go (7)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)pkg/models/validator.go (1)
Validate(16-26)openmeter/ent/db/ent.go (2)
ValidationError(255-258)Value(65-65)openmeter/ent/db/subscription/where.go (2)
Name(106-108)BillingCadence(132-135)openmeter/ent/db/plan/plan.go (3)
FieldBillingCadence(42-42)ValueScanner(134-136)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (3)
FieldBillingCadence(46-46)ValueScanner(152-154)FieldProRatingConfig(48-48)
openmeter/ent/db/plan_update.go (10)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)pkg/models/validator.go (1)
Validate(16-26)openmeter/ent/db/ent.go (2)
ValidationError(255-258)Value(65-65)openmeter/ent/db/plan/where.go (2)
Name(90-92)BillingCadence(120-123)openmeter/ent/db/subscription/where.go (2)
Name(106-108)BillingCadence(132-135)openmeter/ent/db/planratecard/where.go (2)
Name(91-93)BillingCadence(111-114)openmeter/ent/db/plan/plan.go (3)
FieldBillingCadence(42-42)ValueScanner(134-136)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (3)
FieldBillingCadence(46-46)ValueScanner(152-154)FieldProRatingConfig(48-48)openmeter/ent/db/planratecard/planratecard.go (2)
FieldBillingCadence(45-45)ValueScanner(124-129)
api/client/javascript/src/client/schemas.ts (3)
api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingMode(5861-5861)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingMode(14-14)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingMode(5391-5391)
openmeter/ent/db/migrate/schema.go (2)
openmeter/ent/db/plan/plan.go (1)
Columns(81-98)openmeter/ent/db/subscription/subscription.go (1)
Columns(99-116)
openmeter/ent/db/runtime.go (13)
openmeter/ent/db/plan/plan.go (2)
DefaultProRatingConfig(130-130)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (2)
DefaultProRatingConfig(148-148)ValueScanner(152-154)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/billinginvoiceusagebasedlineconfig/billinginvoiceusagebasedlineconfig.go (1)
ValueScanner(64-66)openmeter/ent/db/addonratecard/addonratecard.go (1)
ValueScanner(124-129)openmeter/ent/db/subscriptionitem/subscriptionitem.go (1)
ValueScanner(149-155)openmeter/ent/db/addon/addon.go (1)
ValueScanner(128-130)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
ValueScanner(249-251)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (1)
ValueScanner(107-109)openmeter/ent/db/billinginvoicelineusagediscount/billinginvoicelineusagediscount.go (1)
ValueScanner(96-98)openmeter/ent/db/planratecard/planratecard.go (1)
ValueScanner(124-129)openmeter/ent/db/entitlement/entitlement.go (1)
ValueScanner(165-167)
openmeter/ent/db/mutation.go (6)
pkg/isodate/date.go (1)
String(16-16)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/plan/plan.go (2)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (2)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: CI
- GitHub Check: E2E
- GitHub Check: Analyze (go)
🔇 Additional comments (169)
openmeter/productcatalog/adapter/feature_test.go (1)
322-326: LGTM! Correct implementation of new required fields.The test correctly adds the new required
BillingCadenceandProRatingConfigfields to the plan creation. The values are appropriate:
BillingCadence: "P1M"represents a valid one-month ISO 8601 periodProRatingConfigis properly configured withEnabled: trueand the correct modeThis ensures the test continues to work with the updated plan schema that now requires these fields.
openmeter/productcatalog/plan/serializer_test.go (1)
37-41: LGTM! Proper testing of new field serialization.The test correctly adds the new
BillingCadenceandProRatingConfigfields to thePlanMetastruct:
- Using
isodate.MustParse(t, "P1M")is appropriate for test code as it will fail fast on invalid periods- The
ProRatingConfigvalues are consistent with the domain model- This ensures JSON marshaling/unmarshaling works correctly with the new required fields
The test data is properly structured and validates the serialization behavior.
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql (1)
1-4:✅ Verification successful
Migration rollback script looks correct with data loss warning.
The down migration properly reverses the schema changes by dropping the
pro_rating_configandbilling_cadencecolumns from bothplansandsubscriptionstables.Important considerations:
- This operation will permanently delete all data in these columns
- Ensure you have proper backups before running this rollback
- Verify this rollback matches the corresponding up migration structure
🏁 Script executed:
#!/bin/bash # Description: Verify the corresponding up migration exists and matches this rollback # Expected: Find the up migration file and confirm it adds the columns being dropped fd "20250527114615.*\.up\.sql" tools/migrate/migrations/Length of output: 150
🏁 Script executed:
#!/bin/bash set -eo pipefail file="tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql" echo "Verifying 'plans' table column additions:" rg -n "ALTER TABLE \"plans\" ADD COLUMN \"pro_rating_config\"" "$file" rg -n "ALTER TABLE \"plans\" ADD COLUMN \"billing_cadence\"" "$file" echo "Verifying 'subscriptions' table column additions:" rg -n "ALTER TABLE \"subscriptions\" ADD COLUMN \"pro_rating_config\"" "$file" rg -n "ALTER TABLE \"subscriptions\" ADD COLUMN \"billing_cadence\"" "$file"Length of output: 1233
Migration rollback script validated – matches the up migration
- Confirmed
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sqldrops exactly the same columns that are added in the corresponding up migration (.up.sql).- Down migration drops
pro_rating_config(jsonb) andbilling_cadence(varchar) from both theplansandsubscriptionstables, mirroring the up migration.Important considerations:
- This operation permanently deletes all data in these columns.
- Ensure you have recent backups and test the rollback in a staging environment before applying to production.
openmeter/subscription/subscription.go (1)
9-9: LGTM! Import added for new field support.The import for the
isodatepackage is correctly added to support the newBillingCadencefield.openmeter/productcatalog/subscription/service/plan.go (1)
63-64: Excellent improvement to error handling!The change from
fmt.Errorftomodels.ErrorWithFieldPrefixwith a "plan" field selector provides much better structured error information. This allows API consumers to precisely identify which field caused the validation failure.openmeter/productcatalog/planaddon_test.go (3)
46-54: LGTM! Test data updated for new required fields.The test data correctly includes the new
BillingCadenceandProRatingConfigfields with appropriate values:
BillingCadence: One month period (P1M) - a reasonable defaultProRatingConfig: Enabled with prorate prices mode - covers the main use case
214-222: LGTM! Consistent test data updates.The test data maintains consistency with the "valid" test case, using the same billing cadence and pro-rating configuration values.
409-417: LGTM! Complete test coverage maintained.All test cases now include the required fields, ensuring comprehensive validation coverage for the new billing cadence and pro-rating features.
openmeter/subscription/subscriptionview.go (1)
241-246: LGTM! Proper field mapping in SubscriptionSpec construction.The new
BillingCadenceandProRatingConfigfields are correctly transferred from theSubscriptionto theCreateSubscriptionPlanInput, ensuring data consistency in the subscription view construction.openmeter/subscription/workflow/service/subscription_test.go (2)
28-28: LGTM: Import addition is appropriate.The import for
isodatepackage is correctly added to support theMustParsefunction used in the test data setup.
863-871: LGTM: Test data properly updated with new billing configuration.The addition of
BillingCadenceandProRatingConfigfields to the test plan metadata is consistent with the PR objectives:
BillingCadenceuses the correct ISO duration format "P1M" for monthly billingProRatingConfigis properly configured withEnabled: trueand the correct modeProRatingModeProratePrices- These fields align with the broader system changes to support billing cadence and pro-rating
This ensures the
TestChangeToPlantest covers the new plan metadata fields.openmeter/subscription/workflow/service/subscription.go (1)
144-151: LGTM: Billing cadence validation properly implemented.The billing cadence alignment validation is well-positioned and implemented:
- Correct placement: Validation occurs after customizations are applied but before the database transaction, preventing invalid states from being persisted
- Proper error handling: Uses
models.NewGenericValidationErrorfor consistent validation error responses- Clear error messages: Provides specific error messages for both validation failure and alignment issues
- Business rule enforcement: Ensures billing cadences remain aligned across subscription components
This validation is essential for maintaining data consistency in the billing system.
openmeter/subscription/repository.go (1)
30-34: LGTM: Repository input struct properly extended.The addition of
BillingCadenceandProRatingConfigfields toCreateSubscriptionEntityInputis well-implemented:
- Correct types: Uses appropriate domain types (
isodate.Periodandproductcatalog.ProRatingConfig)- Proper documentation: Clear comments explain the purpose of each field
- Consistent naming: JSON tags follow established conventions
- Logical placement: Fields are positioned appropriately within the struct
These changes align with the broader PR objective to support billing cadence and pro-rating configuration throughout the subscription lifecycle.
openmeter/subscription/aligment_test.go (1)
36-40: LGTM: Test data correctly updated for billing alignment testing.The addition of
BillingCadenceandProRatingConfigto the test plan metadata is appropriate:
- Correct billing cadence: Uses
isodate.MustParse(t, "P1M")for proper ISO duration parsing and monthly billing- Proper pro-rating setup: Enables pro-rating with the correct mode
ProRatingModeProratePrices- Test relevance: These fields are relevant for the
TestAlignedBillingPeriodCalculationtest since billing cadence affects alignment calculations- Consistency: Values match the pattern used in other test files within this PR
This ensures comprehensive test coverage of the new billing configuration features.
test/subscription/scenario_editcancel_test.go (2)
24-24: LGTM! Proper import addition for isodate functionality.The import of the isodate package is correctly added to support the new billing cadence field parsing.
58-65: Excellent addition of billing cadence and pro-rating configuration to test plan metadata.The implementation correctly adds the new required and optional fields:
BillingCadenceusingisodate.MustParsewith "P1M" (1 month duration)ProRatingConfigwith proper structure including enabled flag and prorate prices modeThis aligns perfectly with the PR objectives to add billing cadence and pro-rating support to plans.
openmeter/productcatalog/errors.go (1)
463-470: Well-implemented validation error for billing cadence compatibility.The new validation error properly follows the established pattern in this file:
- Clear error code constant with descriptive name
- Validation issue created using
models.NewValidationIssue- Appropriate warning severity and field targeting
- Descriptive message explaining the compatibility requirement between plan and rate card billing cadences
This will provide clear feedback when billing cadence validation fails.
openmeter/subscription/addon/diff/restore_test.go (2)
23-23: LGTM! Consistent import addition for isodate support.The isodate package import is correctly added to support the billing cadence field functionality.
354-358: Good consistency in test data updates for new billing cadence and pro-rating fields.The test plan metadata is properly updated with:
BillingCadenceset to "P1M" using the appropriateisodate.MustParsefunctionProRatingConfigwith enabled flag and prorate prices modeThis maintains consistency across test files and ensures the addon diff/restore functionality works with the new plan schema.
test/billing/subscription_test.go (2)
115-123: Proper implementation of billing cadence and pro-rating configuration for paid plan.The plan metadata correctly includes:
BillingCadenceusingisodate.MustParsewith "P1D" (1 day duration)ProRatingConfigwith appropriate structure and prorate prices modeThe use of a daily billing cadence is appropriate for this test scenario.
166-174: Consistent addition of billing cadence and pro-rating configuration for free plan.The free plan metadata mirrors the paid plan structure with the same billing cadence and pro-rating configuration, which ensures consistency in the test scenario for default profile change testing.
openmeter/productcatalog/testutils/plan.go (1)
11-11: LGTM! Test utility properly updated for new billing fields.The addition of the
isodateimport and the newBillingCadenceandProRatingConfigfields in the test plan setup is correct. Usingisodate.MustParse(t, "P1M")for a 1-month billing cadence and enabling pro-rating with theProRatingModeProratePricesmode provides sensible defaults for testing.Also applies to: 29-33
openmeter/subscription/repo/subscriptionrepo.go (1)
106-107: LGTM! Repository correctly persists the new billing fields.The addition of
SetBillingCadence(sub.BillingCadence.ISOString())andSetProRatingConfig(sub.ProRatingConfig)to the subscription creation command is correct. Converting the billing cadence to an ISO string for database storage and directly setting the pro-rating config follows the expected persistence patterns.openmeter/productcatalog/plan/service/plan.go (1)
760-762: LGTM! Plan versioning correctly preserves billing configuration.The addition of
Alignment,BillingCadence, andProRatingConfigto the metadata copied when creating the next plan version is correct. This ensures that billing configuration is preserved across plan versions, maintaining consistency for subscription management.openmeter/productcatalog/subscription/plan.go (1)
75-77: LGTM! Plan-to-subscription conversion includes billing configuration.The addition of
Alignment,BillingCadence, andProRatingConfigto theCreateSubscriptionPlanInputconversion is correct. This ensures that billing configuration from plans is properly propagated when creating subscriptions, maintaining consistency between plan definitions and subscription instances.openmeter/subscription/testutils/plan.go (1)
59-67: LGTM! Well-structured test defaults for new billing fields.The addition of
BillingCadenceandProRatingConfigwith sensible defaults (1 month cadence and enabled pro-rating) aligns perfectly with the PR objectives. Usingisodate.MustParsein the test context is appropriate as it provides immediate feedback on invalid ISO 8601 durations.openmeter/productcatalog/plan/adapter/mapping.go (2)
17-21: LGTM! Proper error handling for billing cadence parsing.The implementation correctly parses the billing cadence from the database field and provides informative error messages that include the invalid value for debugging purposes. This follows Go error handling best practices.
39-40: LGTM! Correct assignment of new billing fields to PlanMeta.Both
BillingCadence(parsed) andProRatingConfig(direct assignment) are properly added to thePlanMetastruct, completing the mapping from database entity to domain model.openmeter/productcatalog/plan/service/service_test.go (1)
28-30: LGTM! Improved precision with exact ISO 8601 duration parsing.The refactoring from approximate day-based durations to exact ISO 8601 duration strings ("P1M", "P2M", "P6M") provides better precision and consistency with the billing cadence format used throughout the system. Moving to local scope also reduces global state.
openmeter/subscription/repo/mapping.go (2)
31-34: LGTM! Proper error handling for billing cadence parsing.The parsing of
BillingCadencefrom the database format (isodate.String) to the domain model format (isodate.Period) is correctly implemented with appropriate error handling and context.
56-62: LGTM! Clean mapping of new fields to domain model.The mapping of
BillingCadenceandProRatingConfigfields from the database entity to the domain subscription model is correctly implemented and consistent with the existing field mapping pattern.openmeter/subscription/workflow/service/addon_test.go (2)
27-27: LGTM! Necessary import addition.The
isodateimport is correctly added to support the newMustParsefunction usage in the test.
571-575: LGTM! Appropriate test data enhancement.The addition of
BillingCadence(monthly, "P1M") andProRatingConfig(enabled with prorate_prices mode) to the test plan metadata aligns well with the new schema requirements and uses realistic values for testing.test/customer/customer.go (3)
22-22: LGTM! Required import for new functionality.The
isodateimport is correctly added to support parsing of billing cadence periods in the test plan metadata.
226-232: LGTM! Consistent test data enhancement.The addition of
BillingCadenceandProRatingConfigto the test plan metadata is consistent with the new schema requirements and uses appropriate values for testing customer scenarios with subscriptions.
561-567: LGTM! Consistent test data across test functions.The billing cadence and pro-rating configuration values are consistently applied across both test functions that require plan metadata, maintaining good test data consistency.
api/spec/src/productcatalog/prorating.tsp (1)
1-31: LGTM! Well-structured TypeSpec definitions for pro-rating.The pro-rating types are well-defined with:
- Clear namespace organization under
OpenMeter.ProductCatalog- Extensible union type for
ProRatingModethat can accommodate future modes- Sensible defaults for
ProRatingConfig(enabled: true, mode: proratePrices)- Comprehensive documentation with annotations and comments
The design allows for future extensibility while providing a solid foundation for pro-rating functionality.
openmeter/ent/db/predicate/predicate.go (2)
215-224: LGTM! Consistent error handling pattern for Plan predicates.The
PlanOrErrfunction follows the established pattern of other*OrErrfunctions in this file, enabling error-aware predicate usage for Plan entities.Note: There's a consistent typo in the comment "not nit" (should be "not nil") that exists across multiple functions in this file, but addressing it would be outside the scope of this PR.
249-258: LGTM! Consistent error handling pattern for Subscription predicates.The
SubscriptionOrErrfunction follows the established pattern and enables error-aware predicate usage for Subscription entities, complementing the Plan predicate changes.test/subscription/scenario_editaligned_test.go (3)
23-23: LGTM! Import added for isodate support.The isodate package import is correctly added to support the new
BillingCadencefield in the test plan metadata.
57-64: LGTM! Appropriate test data for billing cadence and pro-rating.The test plan metadata is correctly enhanced with:
BillingCadenceset to "P1M" (1 month) using the appropriateisodate.MustParsehelperProRatingConfigwith sensible defaults (enabled with prorate_prices mode)This aligns well with the broader PR objectives of adding billing cadence and pro-rating support.
238-245: LGTM! Consistent test data across test functions.The second test function is updated with identical billing cadence and pro-rating configuration, ensuring consistency between the aligned and unaligned subscription test scenarios.
openmeter/productcatalog/plan/adapter/plan.go (2)
183-184: LGTM! Correct implementation of new fields in plan creation.The billing cadence and pro-rating configuration fields are properly set during plan creation:
BillingCadencecorrectly usesISOString()to convert the isodate to string formatProRatingConfigis set directly as a structured configuration objectThis follows the established pattern for other plan fields.
428-430: LGTM! Proper handling of nullable fields in plan updates.The plan update correctly handles the new fields as nullable updates:
BillingCadenceusesISOStringPtrOrNil()for proper null handling during updatesProRatingConfigis set as nullable to allow clearing the configurationBillablesMustAlignis consistently moved to nullable setter as wellThis pattern aligns with how other optional fields are handled in updates.
pkg/isodate/date_test.go (1)
81-217: LGTM! Comprehensive test coverage for period divisibility.The
TestDivisibleByfunction provides excellent test coverage with:Strengths:
- Well-structured table-driven tests with clear test case descriptions
- Comprehensive coverage of both compatible and incompatible period combinations
- Tests various time units (years, months, weeks, days) and their divisibility relationships
- Includes edge cases like same periods and non-divisible combinations
- Proper error handling validation with
hasErrorflagTest scenarios covered:
- Year divisibility by months (1Y ÷ 1M, 2M, 3M, 4M, 6M)
- Month divisibility by days and weeks
- Week divisibility scenarios
- Incompatible combinations (month ÷ week, year ÷ 5 months, etc.)
This thorough testing ensures the reliability of the billing cadence alignment logic that depends on the
DivisibleBymethod.openmeter/productcatalog/addon/adapter/mapping.go (2)
187-190: LGTM! Proper error handling for billing cadence parsing.The billing cadence parsing follows the established pattern in this codebase with appropriate error handling and type conversion from
entdb.Plan.BillingCadenceto the domain type.
207-208: LGTM! Consistent field assignment pattern.The assignment of both
BillingCadenceandProRatingConfigto thePlanMetastruct follows the existing code structure and properly integrates the parsed values.api/spec/src/productcatalog/subscription.tsp (2)
95-104: LGTM! Well-documented billing cadence property.The
billingCadenceproperty is properly defined with:
- Clear documentation explaining its purpose and format
- Correct ISO8601 duration encoding
- Appropriate example ("P1M" for monthly)
- Read-only visibility annotation
This follows TypeSpec best practices and aligns with the domain requirements.
106-114: LGTM! Sensible default for pro-rating configuration.The
proRatingConfigproperty is well-defined with:
- Optional typing with a reasonable default configuration
- Enabled by default with
proratePricesmode- Read-only visibility for API consumers
- Clear documentation
The default configuration promotes pro-rating by default, which is typically beneficial for users.
openmeter/ent/db/subscription.go (5)
16-18: LGTM! Proper imports for new field types.The imports for
productcatalogandisodatepackages are correctly added to support the newBillingCadenceandProRatingConfigfields.
52-55: LGTM! Well-documented new fields.The new fields are properly defined with:
BillingCadenceasisodate.StringtypeProRatingConfigasproductcatalog.ProRatingConfigtype- Clear field comments explaining their purpose
- Consistent JSON tags following the existing pattern
137-142: LGTM! Proper scanning configuration.The
scanValuesmethod correctly handles the new fields:
BillingCadencetreated as string (line 137)ProRatingConfiguses the specializedValueScanner.ProRatingConfig.ScanValue()(line 142)This follows the established pattern for complex types in ent.
248-259: LGTM! Correct value assignment logic.The
assignValuesmethod properly handles both new fields:
BillingCadence: Standard string-to-isodate.String conversion (lines 248-253)ProRatingConfig: Uses specialized scanner with proper error handling (lines 254-259)The error handling and type conversions follow ent conventions.
367-372: LGTM! Complete string representation.The
String()method properly includes both new fields in the output, maintaining consistency with the existing field formatting pattern.tools/migrate/entitlement_test.go (1)
21-22: LGTM! Improved struct initialization clarity.The explicit naming of the
stopsfield in therunnerstruct initialization improves code readability and makes the structure more explicit. This change is consistently applied across all three test functions and follows Go best practices for struct initialization.Also applies to: 138-139, 495-496
openmeter/productcatalog/plan/service.go (3)
13-13: LGTM - Import addition is appropriate.The import for
pkg/isodateis correctly added to support the newBillingCadencefield.
155-159: LGTM - New fields are properly structured.The new optional fields are well-designed:
- Both fields use pointer types, making them truly optional
- Field names and JSON tags follow consistent naming conventions
- Comments provide clear descriptions of their purpose
199-206:✅ Verification successful
Verify the equality comparison logic.
The equality comparisons look correct, but please verify:
- That
Planstruct has correspondingBillingCadenceandProRatingConfigfields- That
ProRatingConfighas anEqualmethod implementation
🏁 Script executed:
#!/bin/bash # Description: Verify that Plan struct has the corresponding fields and ProRatingConfig has Equal method # Check Plan struct fields ast-grep --pattern $'type Plan struct { $$$ BillingCadence $_ $$$ ProRatingConfig $_ $$$ }' # Check ProRatingConfig Equal method ast-grep --pattern $'func ($_) Equal($_) bool { $$$ }'Length of output: 68442
Equality checks in UpdatePlanInput.Equal are correct
I’ve confirmed that in
openmeter/productcatalog/plan/service.gothePlantype (via its embeddedPlanMeta) exposes bothBillingCadenceandProRatingConfigfields, and thatProRatingConfig.Equalis implemented in
• openmeter/productcatalog/pro_rating.go:46.No changes required.
openmeter/ent/schema/subscription.go (2)
40-42: LGTM - Billing cadence field is well-configured.The
billing_cadencefield is properly configured with:
- Appropriate type (
isodate.String)- Clear comment describing its purpose
43-55:❓ Verification inconclusive
Verify ProRatingConfigValueScanner availability and consider field requirements.
The
pro_rating_configfield configuration looks comprehensive, but there are a few concerns:
- ValueScanner Reference:
ProRatingConfigValueScannershould be defined in this package or imported- Default Configuration: The default enables pro-rating by default - verify this aligns with business requirements
- Field Requirements: Both new fields are not marked as
Optional()orNillable(), making them requiredConsider making the fields optional if they should allow nil values:
field.String("billing_cadence"). GoType(isodate.String("")). + Optional(). Comment("The default billing cadence for subscriptions."), field.String("pro_rating_config"). GoType(productcatalog.ProRatingConfig{}). ValueScanner(ProRatingConfigValueScanner). + Optional(). DefaultFunc(func() productcatalog.ProRatingConfig {
🏁 Script executed:
#!/bin/bash # Description: Verify ProRatingConfigValueScanner definition and imports # Check if ProRatingConfigValueScanner is defined in this file or imported rg -A 3 -B 3 "ProRatingConfigValueScanner" # Check if it's defined in the productcatalog schema file ast-grep --pattern $'ProRatingConfigValueScanner = $_'Length of output: 2042
Verify ProRatingConfigValueScanner import and field optionality
- ProRatingConfigValueScanner is defined in openmeter/ent/schema/productcatalog.go via
ProRatingConfigValueScanner = entutils.JSONStringValueScanner[productcatalog.ProRatingConfig]().
Ensure theproductcatalogpackage is imported insubscription.go.- Confirm that enabling pro-rating by default (
Enabled: true) matches your business requirements.- Both
billing_cadenceandpro_rating_configare currently required (no.Optional()/.Nillable()). If they should accept nil or be optional on creation, add.Optional()(or.Nillable()).Suggested diff:
field.String("billing_cadence"). GoType(isodate.String("")). + Optional(). Comment("The default billing cadence for subscriptions."), field.String("pro_rating_config"). GoType(productcatalog.ProRatingConfig{}). ValueScanner(ProRatingConfigValueScanner). + Optional(). DefaultFunc(func() productcatalog.ProRatingConfig { return productcatalog.ProRatingConfig{ Mode: productcatalog.ProRatingModeProratePrices, Enabled: true, } }). SchemaType(map[string]string{ dialect.Postgres: "jsonb", }). Comment("Default pro-rating configuration for subscriptions."),openmeter/billing/worker/subscription/sync_test.go (4)
157-165: LGTM! Systematic test data enhancement for new billing features.The addition of
BillingCadenceandProRatingConfigfields to the test plan data is well-structured and aligns with the broader system changes introducing billing cadence and pro-rating support. The monthly cadence (P1M) and pro-rating configuration are appropriate for this test scenario.
574-582: LGTM! Consistent test data updates.The billing cadence and pro-rating configuration additions are consistent with the pattern established in other test scenarios, ensuring comprehensive test coverage of the new billing features.
1391-1395: LGTM! Comprehensive test coverage for new billing features.These systematic additions of
BillingCadenceandProRatingConfigfields across multiple test scenarios ensure that:
- All test plans include the newly required billing cadence field
- Pro-rating functionality is consistently tested with appropriate configuration
- Different billing cadences (P1M, P2W) are tested across various scenarios
- The test suite remains compatible with the enhanced product catalog schema
The values are well-chosen for each test context and maintain consistency in the pro-rating configuration approach.
Also applies to: 1467-1471, 1690-1694, 1859-1863
3806-3814: LGTM! Final test data enhancement completes the pattern.This addition in the
createSubscriptionFromPlanPhaseshelper function ensures that all dynamically created test plans include the new billing cadence and pro-rating configuration fields, providing consistent test coverage across the entire test suite.api/spec/src/productcatalog/plan.tsp (2)
4-4: LGTM! Proper import added for pro-rating types.The import statement correctly includes the prorating.tsp module to support the new ProRatingConfig field.
83-102: Excellent schema design for billing cadence and pro-rating configuration.The new fields are well-designed with:
billingCadence: Required field with proper ISO8601 duration encoding and clear examplesproRatingConfig: Optional field with sensible default values (enabled=true, mode=prorate_prices)- Comprehensive documentation and appropriate visibility annotations
- Consistent with TypeSpec conventions
The default pro-rating configuration provides a good out-of-the-box experience while allowing customization.
e2e/productcatalog_test.go (2)
210-243: Comprehensive test coverage for new billing fields.The test updates properly include:
BillingCadenceset to "P1M" (monthly) in plan creationProRatingConfigwith appropriate mode and enabled flag for custom plans- Consistent field usage across different test scenarios
This ensures the new features are properly validated in end-to-end testing.
439-441: Excellent assertions for verifying billing configuration.The test assertions properly verify that:
- Created subscriptions inherit the correct billing cadence from their plans
- Pro-rating configuration is correctly applied with the expected mode and enabled state
- Both custom subscriptions and plan-based subscriptions maintain consistent behavior
This provides confidence that the billing cadence and pro-rating features work correctly across different subscription creation paths.
Also applies to: 497-499
openmeter/ent/db/plan.go (2)
14-15: Proper imports and field additions for new billing features.The generated Ent code correctly:
- Imports required packages for
isodateandproductcatalogtypes- Adds
BillingCadenceasisodate.StringandProRatingConfigasproductcatalog.ProRatingConfig- Maintains consistent field documentation and JSON tags
The type choices are appropriate for the domain requirements.
Also applies to: 45-48
110-111: Correct database scanning and assignment logic.The generated code properly handles the new fields:
BillingCadenceuses standard string scanning for ISO duration formatProRatingConfigusesValueScannerfor complex type handling- Assignment logic correctly handles null values and type conversion
- Error handling is appropriate for database operations
This ensures reliable persistence and retrieval of billing configuration data.
Also applies to: 114-115, 207-218
openmeter/ent/db/plan/plan.go (2)
41-44: Complete field constant and column definitions.The generated constants properly define:
- Field names matching database column naming conventions
- Inclusion in the
Columnsslice for SQL operations- Consistent naming patterns with existing fields
This ensures proper SQL query generation and field validation.
Also applies to: 94-95
129-136: Comprehensive support for new field operations.The generated code provides complete functionality:
DefaultProRatingConfigfunction for proper field initializationValueScannerfor complex type database operations- Ordering functions
ByBillingCadenceandByProRatingConfigfor query sortingThis enables full CRUD operations and query flexibility for the new billing fields.
Also applies to: 197-205
openmeter/ent/schema/productcatalog.go (3)
36-38: LGTM! The billing cadence field is properly configured.The field correctly uses
isodate.Stringas the Go type for ISO duration strings and includes a descriptive comment.
39-51: Well-structured pro-rating config field with sensible defaults.The field configuration is comprehensive with:
- Appropriate JSONB type for PostgreSQL
- Custom value scanner for proper serialization
- Default enabled pro-rating for backward compatibility
- Clear documentation
200-200: Value scanner correctly defined.The
ProRatingConfigValueScannerfollows the established pattern and is properly typed for the non-pointerproductcatalog.ProRatingConfigtype.openmeter/ent/db/subscription/subscription.go (3)
45-48: Generated field constants look correct.The new field constants follow the established naming convention and are properly documented.
147-154: Default function and value scanner properly configured.The generated code correctly adds the default function type and includes the ProRatingConfig field in the ValueScanner struct, following the established patterns.
225-233: Ordering functions correctly implemented.The generated ordering functions for both fields follow the established pattern and will enable proper query ordering.
openmeter/productcatalog/plan/httpdriver/mapping.go (4)
33-34: API response mapping correctly handles new fields.The function properly converts both fields to their API representations using appropriate methods.
105-105: Create request properly handles new fields with validation.The implementation correctly:
- Uses the helper function for pro-rating config conversion
- Parses and validates the billing cadence with appropriate error handling
Also applies to: 126-129
147-169: Helper functions provide clean bidirectional conversion.The implementation is well-structured:
- Clean separation of concerns with dedicated conversion functions
- Proper nil handling with sensible defaults for backward compatibility
- Consistent type conversions between API and domain models
205-205: Update request correctly handles optional field updates.The implementation properly:
- Uses the helper function with pointer wrapper for optional pro-rating config
- Only parses billing cadence when provided (non-empty string)
- Includes appropriate error handling for the parsing operation
Also applies to: 214-221
openmeter/productcatalog/pro_rating.go (5)
1-9: LGTM! Clean package structure and appropriate imports.The package declaration and imports are well-organized, using the necessary dependencies for validation and error handling.
10-21: Good enum pattern implementation.The ProRatingMode type with the Values() method provides a clean enumeration pattern that's consistent with Go conventions and useful for validation.
22-29: Well-structured configuration type.The ProRatingConfig struct is simple and clear with appropriate JSON tags and documentation. The field types are suitable for the use case.
31-43: Solid validation implementation.The validation method follows good patterns by collecting errors and using the established validation helpers from the models package. Only validating the Mode field makes sense since Enabled is a boolean that can't be invalid.
45-48: Simple and correct equality method.The Equal method provides a straightforward comparison of both fields, which is appropriate for this value type.
openmeter/subscription/subscriptionspec.go (3)
35-39: Well-documented new fields added to subscription input.The BillingCadence and ProRatingConfig fields are properly added with clear documentation and appropriate types. The JSON tags are consistent with the existing pattern.
65-73: Correct mapping of new fields in entity conversion.The ToCreateSubscriptionEntityInput method properly includes the new BillingCadence and ProRatingConfig fields in the entity input mapping.
314-347: Robust billing cadence alignment validation.The HasAlignedBillingCadences method implements comprehensive validation logic:
- Iterates through all phases and billable items
- Handles three comparison cases (equal, greater, less) correctly
- Uses divisibility checks for non-equal cadences
- Returns early on misalignment for efficiency
The logic is sound and follows the requirements for billing cadence alignment validation.
openmeter/productcatalog/subscription/http/mapping.go (3)
196-200: Proper API mapping for new subscription fields.The BillingCadence and ProRatingConfig fields are correctly mapped to the API response with appropriate type conversions (using String() method for Period and mapping struct fields).
456-479: Good error handling for billing cadence parsing.The BillingCadence parsing includes proper error handling for invalid ISO duration strings, and the ProRatingConfig is set using the helper function. The error message provides clear context.
497-510: Sensible default configuration handling.The asProRatingConfig helper function provides a reasonable default when ProRatingConfig is nil (enabled with the only available mode), and correctly maps the fields when provided. This ensures backward compatibility.
openmeter/ent/db/migrate/schema.go (3)
1730-1731: Well-structured schema additions for billing configuration.The new
billing_cadenceandpro_rating_configcolumns are properly defined with appropriate types. The JSONB schema type forpro_rating_configis well-suited for flexible configuration storage.
2000-2001: Consistent schema additions across plans and subscriptions.The column definitions match the plans table structure, ensuring consistency in how billing cadence and pro-rating configuration are handled across both entities.
2013-2043:❓ Verification inconclusive
Verify the foreign key column index updates are correct.
The foreign key column references have been updated to account for the two new columns. Let me verify the index calculations:
- Before:
customer_idat index 12,plan_idat index 13- After: With two new columns inserted,
customer_idshould be at index 14,plan_idat index 15Looking at the SubscriptionsColumns definition, the new columns are inserted at indices 12-13, pushing customer_id and plan_id to indices 14-15. The updates appear correct.
Let me verify the column index calculations are accurate:
🏁 Script executed:
#!/bin/bash # Description: Verify the column positions in the subscriptions table schema # Expected: customer_id at index 14, plan_id at index 15 echo "Checking SubscriptionsColumns indices:" grep -n "customer_id\|plan_id\|billing_cadence\|pro_rating_config" openmeter/ent/db/migrate/schema.go | grep -A 5 -B 5 "SubscriptionsColumns"Length of output: 228
🏁 Script executed:
#!/usr/bin/env python3 import re, sys file_path = "openmeter/ent/db/migrate/schema.go" cols = [] in_block = False with open(file_path) as f: for line in f: if not in_block: if re.search(r'var\s+SubscriptionsColumns', line): in_block = True continue # Stop when the next columns slice starts if re.search(r'var\s+PlansColumns', line): break # Capture column names m = re.search(r'Name:\s*"([^"]+)"', line) if m: cols.append(m.group(1)) print(f"Total columns: {len(cols)}") for idx, name in enumerate(cols): print(f"{idx}: {name}")Length of output: 699
#!/usr/bin/env bash # Extract and index SubscriptionColumns Name entries file="openmeter/ent/db/migrate/schema.go" if [[ ! -f "$file" ]]; then echo "File not found: $file" >&2 exit 1 fi echo "Listing SubscriptionsColumns with indices:" idx=0 awk '/SubscriptionsColumns = \[\*\]schema\.Column{/,/}/' "$file" \ | grep 'Name:' \ | while read -r line; do name=$(echo "$line" | sed -E 's/.*Name:[[:space:]]*"([^"]+)".*/\1/') printf "%2d: %s\n" "$idx" "$name" idx=$((idx+1)) done
#!/usr/bin/env bash # Extract and index SubscriptionsColumns names to verify positions file="openmeter/ent/db/migrate/schema.go" if [[ ! -f "$file" ]]; then echo "Error: File not found: $file" >&2 exit 1 fi in_slice=0 brace=0 index=0 echo "SubscriptionsColumns indices:" while IFS= read -r line; do if [[ $in_slice -eq 0 ]]; then if [[ "$line" =~ var[[:space:]]+SubscriptionsColumns ]]; then in_slice=1 # Initialize brace count at first '{' if [[ "$line" == *"{"* ]]; then brace=1 fi fi continue fi # Count braces to detect end of slice opens=$(grep -o "{" <<< "$line" | wc -l) closes=$(grep -o "}" <<< "$line" | wc -l) brace=$((brace + opens - closes)) # Print column names with their index if [[ "$line" =~ Name: ]]; then name=$(sed -E 's/.*Name:[[:space:]]*"([^"]+)".*/\1/' <<< "$line") printf "%2d: %s\n" "$index" "$name" index=$((index+1)) fi # Exit when we've closed the slice if [[ $brace -eq 0 ]]; then break fi done < "$file"
Manual verification needed: Confirm foreign key column indices
I wasn’t able to auto-verify the
SubscriptionsColumnsordering via script. Please double-check that:
SubscriptionsColumns[14]iscustomer_idSubscriptionsColumns[15]isplan_idto ensure the new columns correctly shifted the indexes.
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql (6)
1-3: Safe migration pattern: add nullable columns first.Adding columns as nullable initially is the correct approach to avoid immediate constraint violations during the migration process.
7-32: Complex but well-structured billing cadence extraction logic.The CTE approach to extract billing cadence from plan rate cards is sound:
- Filters for the last phase (
duration IS NULL) and excludes soft-deleted records- Uses window functions to find the shortest billing cadence per plan
- Handles NULL and empty billing cadence values appropriately
The
billing_cadence::intervalcasting enables proper chronological comparison for finding the shortest (most frequent) cadence.
34-38: Appropriate default handling for plans without billing cadence.Setting
P1M(monthly) as the default billing cadence is a reasonable fallback. The comment correctly notes that defaults are applied to ALL plans, including soft-deleted ones, which is necessary to satisfy the upcoming NOT NULL constraint.
56-86: Well-designed subscription billing cadence extraction.The subscription migration logic correctly:
- Uses ROW_NUMBER() to identify the last phase by
active_from DESC- Extracts billing cadence from subscription items in the last phase
- Applies similar filtering and shortest-cadence selection as the plans migration
- Properly handles soft-deleted records
The approach is consistent with the plans migration logic while accounting for the different schema structure of subscriptions.
88-102: Consistent default handling and constraint application.The subscription defaults mirror the plans table approach:
P1Mfor billing cadence- Same pro-rating configuration JSON
- Applied to all records including soft-deleted ones
- Columns made NOT NULL after population
This ensures schema consistency between plans and subscriptions.
40-44:✅ Verification successful
Verify the default pro-rating configuration structure.
The default JSON structure
{"mode": "prorate_prices", "enabled": true}should align with the expected pro-rating configuration schema used elsewhere in the codebase.Let me verify this default configuration matches the expected schema:
🏁 Script executed:
#!/bin/bash # Description: Check if the default pro-rating config structure matches the expected schema # Expected: The JSON structure should align with ProRatingConfig type definitions echo "Searching for ProRatingConfig type definitions and validation:" rg -A 10 "type.*ProRatingConfig" --type go echo "---" echo "Searching for pro-rating mode values:" rg "prorate_prices|ProRatingMode" --type go -A 3 -B 3Length of output: 35337
Default pro-rating configuration JSON is correct.
The migration’sUPDATE plans SET pro_rating_config = '{"mode": "prorate_prices", "enabled": true}'aligns with theProRatingConfigdefinition and the constantProRatingModeProratePrices = "prorate_prices". Both unit and e2e tests (e.g.tools/migrate/productcatalog_test.go, subscription tests, HTTP mappers, and Ent schemas) confirm thatenabled: trueandmode: "prorate_prices"are parsed and validated correctly. No changes required.pkg/isodate/date.go (1)
118-144: Well-designed divisibility check for periods.The implementation correctly handles the complexity of variable month lengths by testing divisibility across all possible days-per-month values (28-31). This ensures accurate validation of billing cadence alignment regardless of the specific months involved.
openmeter/productcatalog/plan.go (1)
243-249: Proper validation for new billing configuration fields.The validation correctly ensures that
BillingCadenceis not empty and thatProRatingConfigpasses its own validation rules. The error messages are clear and descriptive.openmeter/ent/db/subscription_update.go (1)
358-362: Correct validation implementation for ProRatingConfig.The generated code properly validates the
ProRatingConfigfield by calling itsValidate()method and wrapping any errors with appropriate context. This ensures data integrity when updating subscriptions.Also applies to: 942-946
openmeter/ent/db/plan_update.go (7)
19-20: LGTM! Import additions are correct.The new imports for
productcatalogandisodatepackages are properly added to support the new billing cadence and pro-rating configuration fields.
143-169: LGTM! Setter methods follow established patterns.The new setter methods for
billing_cadenceandpro_rating_configfields are implemented correctly:
- Regular setters properly delegate to the mutation object
- Nillable setters include proper nil checks
- Method chaining is maintained by returning the builder
- Implementation is consistent with existing field setters
367-371: LGTM! Validation logic is properly implemented.The ProRatingConfig validation correctly:
- Only validates when the field is being set
- Delegates to the domain object's
Validate()method- Provides clear error context with proper field name
- Follows the established validation pattern used by other fields
420-429: LGTM! SQL save field handling is correctly implemented.The new field handling in
sqlSaveis well-implemented:
billing_cadenceis properly handled as a string fieldpro_rating_configcorrectly uses the ValueScanner for database serialization- Error handling for value conversion is robust and immediate
- Both implementations follow the established pattern used by other fields
704-730: LGTM! PlanUpdateOne setter methods are correctly implemented.The setter methods for
PlanUpdateOneproperly mirror thePlanUpdateimplementations:
- Consistent method signatures and behavior
- Proper nil checking in nillable setters
- Method chaining maintained
- Implementation follows the established pattern
941-945: LGTM! PlanUpdateOne validation logic is correctly implemented.The validation logic properly mirrors the
PlanUpdateimplementation, ensuring consistent behavior across both builder types.
1011-1020: LGTM! PlanUpdateOne SQL save field handling is correctly implemented.The field handling in
PlanUpdateOne.sqlSaveproperly mirrors thePlanUpdateimplementation:
- Consistent field setting logic
- Proper value scanner usage for ProRatingConfig
- Robust error handling
- Maintains the established pattern
api/openapi.cloud.yaml (12)
13156-13156: Required property inclusion is correct.
You've addedbillingCadenceto therequiredlist for this plan input schema, ensuring clients must supply the new field.
19127-19127: Schema consistency check passed.
ThebillingCadenceentry has been correctly added to therequiredlist for this plan version schema.
19202-19218: Approve new plan output schema fields.
ThebillingCadenceandproRatingConfigproperties are well-defined with clear descriptions, examples, and default values.
19419-19419: Required property in plan update schema is correct.
billingCadencehas been added to therequiredlist for the plan replace/update schema.
19457-19473: Approve billingCadence and proRatingConfig definitions.
The property definitions mirror other plan schemas, maintaining consistency across the API spec.
19607-19607: Required field addition is accurate.
billingCadenceis properly marked as required in this plan alignment schema.
19632-19648: Approve schema enhancements.
NewbillingCadenceandproRatingConfigfields align with existing schema patterns and include appropriate metadata.
19840-19862: New ProRatingConfig and ProRatingMode schemas look good.
The definitions include required fields, defaults, and enum constraints, providing a clear contract for pro-rating behavior.
20851-20851: Required property in subscription listing is correct.
billingCadencehas been added to therequiredlist of the Subscription response schema.
20935-20953: Approve subscription schema updates.
billingCadenceandproRatingConfigare correctly defined withreadOnly: true, matching design intent.
21326-21326: Required field inclusion is correct.
billingCadencehas been appropriately added to the required list for the expanded Subscription schema.
21407-21425: Approve subscription expanded schema fields.
Fields match the definitions used elsewhere and maintain consistent behavior for pro-rating and cadence configuration.api/client/javascript/src/client/schemas.ts (7)
7954-7971: Schema definitions are consistent and well-documented.The billing configuration fields are properly added to this plan schema with consistent documentation, types, and default values matching the previous schema definitions.
8166-8183: Good consistency across plan schemas.The field definitions, documentation, and types are consistent with other plan schemas in the file, maintaining uniformity across the API surface.
8289-8306: CustomPlanInput schema properly updated.The billing cadence and pro-rating fields are correctly added with consistent documentation and typing. The required
billingCadencemaintains consistency with other plan creation schemas.
8433-8452: Well-structured type definitions for pro-rating configuration.The
ProRatingConfigandProRatingModeschemas are properly defined with:
- Clear documentation for both the configuration object and individual fields
- Appropriate default values (enabled: true, mode: 'prorate_prices')
- Proper enum constraint for
ProRatingModelimiting it to 'prorate_prices'This aligns well with the Go type definitions shown in the relevant code snippets.
9297-9314: Subscription schema correctly inherits billing configuration.The fields are appropriately marked as
readonlyin the subscription schema, indicating these values are inherited from the plan and not directly modifiable on the subscription. The documentation correctly notes this is "for the subscriptions" rather than "for the plan".
9666-9683: SubscriptionExpanded schema maintains consistency.The billing configuration fields are properly added to the expanded subscription schema with consistent readonly semantics and documentation.
10915-10916: Export statements properly added for new types.The new
ProRatingConfigandProRatingModetypes are correctly exported, making them available for consumption by client code.tools/migrate/productcatalog_test.go (2)
15-61: LGTM! Well-structured migration test.The test correctly validates the plan phase deletion during the migration from
start_aftertodurationformat.
63-178: LGTM! Comprehensive ISO format migration test.The test thoroughly validates:
- Conversion of legacy formats (e.g., 'MONTH' → 'P1M')
- Soft deletion of incompatible formats
- Proper cleanup in down migration
openmeter/ent/db/plan/where.go (1)
1-927: Generated code looks correct.The predicate functions for the
billing_cadencefield follow the established patterns and properly handle theisodate.Stringtype conversion.api/openapi.yaml (3)
20264-20287: New schema definitions look correct
TheProRatingConfigandProRatingModeschemas are well-defined, with required fields, clear descriptions, and default preferences. This aligns with the desired functionality for pro-rating.
21469-21478: Schema definition for Subscription.billingCadence
ThebillingCadenceproperty is correctly defined withformat: durationand markedreadOnlyfor response schemas. Ensure consistency of examples and descriptions across subscription endpoints.
21941-21959: SubscriptionExpanded properties defined correctly
ThebillingCadenceandproRatingConfigfields are properly defined and markedreadOnly. The defaults and examples align with other response schemas.openmeter/ent/db/subscription/where.go (3)
12-12: LGTM: Import correctly added for billing cadence support.The
isodatepackage import is properly added to support the new billing cadence predicate functions that useisodate.Stringtype.
131-135: LGTM: Shorthand function correctly implemented.The
BillingCadenceshorthand function follows the established pattern and correctly convertsisodate.Stringto string for SQL operations.
806-888: LGTM: Comprehensive predicate functions correctly generated.The billing cadence predicate functions are properly implemented following the established Ent ORM patterns. All functions correctly:
- Accept
isodate.Stringparameters- Convert to string for SQL operations
- Handle slice operations appropriately for In/NotIn predicates
- Follow the same structure as other field predicates
The generated code provides complete SQL query capabilities for the billing cadence field.
openmeter/ent/db/mutation.go (16)
38672-38673: LGTM: Field declarations follow ENT conventions.The new field declarations for
billing_cadenceandpro_rating_configare correctly typed and follow the established pattern for optional fields in the PlanMutation struct.
39250-39320: LGTM: Complete method implementations for new Plan fields.The getter, setter, old value retrieval, and reset methods for both
billing_cadenceandpro_rating_configfields are properly implemented following ENT patterns. Error handling and method signatures are consistent with existing field methods.
39616-39616: LGTM: Field count updated correctly.The field count has been properly updated from 13 to 15 to accommodate the two new fields being added to the PlanMutation.
39650-39655: LGTM: New fields properly integrated into Fields() method.Both
billing_cadenceandpro_rating_configfields are correctly added to the field enumeration logic, maintaining consistency with the existing pattern.
39692-39695: LGTM: Field() method updated correctly.The Field() method properly handles the new fields with appropriate case statements that delegate to the respective getter methods.
39731-39734: LGTM: OldField() method integration.The OldField() method correctly includes the new fields and delegates to their respective old value retrieval methods.
39825-39838: LGTM: SetField() method with proper type validation.The SetField() method correctly handles the new fields with appropriate type assertions and error handling. The type validation ensures type safety at runtime.
39983-39988: LGTM: ResetField() method integration.The ResetField() method properly includes the new fields and delegates to their respective reset methods.
43814-43815: LGTM: Subscription field declarations.The new field declarations in SubscriptionMutation mirror the PlanMutation structure correctly, maintaining consistency across both entities.
44472-44542: LGTM: Complete method implementations for new Subscription fields.The implementation for SubscriptionMutation fields follows the exact same pattern as PlanMutation, ensuring consistency across the codebase. All methods are properly implemented with appropriate error handling.
44794-44794: LGTM: Subscription field count updated.The field count for SubscriptionMutation has been properly updated to 15, consistent with the PlanMutation changes.
44834-44839: LGTM: Subscription Fields() method integration.Both new fields are correctly added to the SubscriptionMutation Fields() method, maintaining the established pattern.
44874-44877: LGTM: Subscription Field() method updated.The Field() method for SubscriptionMutation properly handles the new fields with correct case statements.
44913-44916: LGTM: Subscription OldField() method integration.The OldField() method correctly includes the new fields for SubscriptionMutation.
45017-45030: LGTM: Subscription SetField() method with type validation.The SetField() method for SubscriptionMutation correctly handles the new fields with proper type assertions and error handling.
45152-45157: LGTM: Subscription ResetField() method integration.The ResetField() method for SubscriptionMutation properly includes the new fields and delegates to their respective reset methods.
openmeter/ent/db/subscription_create.go (3)
176-194: LGTM! Setter methods properly implemented.The setter methods for
billing_cadenceandpro_rating_configfollow the established patterns. The nillable setter forpro_rating_configis appropriately included since it has a default value.
469-480: Field serialization properly implemented.The code correctly handles both fields in
createSpec(), including proper value conversion forpro_rating_configusing the ValueScanner with appropriate error handling.
741-763: Upsert methods correctly implemented.All upsert builders (
SubscriptionUpsert,SubscriptionUpsertOne, andSubscriptionUpsertBulk) are properly extended with methods to handlebilling_cadenceandpro_rating_configduring upsert operations.Also applies to: 975-1001, 1383-1409
openmeter/ent/db/plan_create.go (4)
145-163: LGTM! Setter methods follow established patterns.The setter methods for
billing_cadenceandpro_rating_configare correctly implemented with appropriate nillable setter forpro_rating_config.
359-369: Validation properly enforces field requirements.Both
billing_cadenceandpro_rating_configare correctly validated as required fields, withpro_rating_confighaving additional validation through itsValidate()method.
453-464: Database field handling correctly implemented.The
createSpec()method properly handles both fields with appropriate value conversion forpro_rating_configand error handling.
681-703: Upsert operations properly support new fields.All upsert builders are correctly extended with methods for
billing_cadenceandpro_rating_config, maintaining consistency across the codebase.Also applies to: 927-953, 1353-1379
api/client/javascript/src/zod/index.ts (6)
10076-10081: LGTM! Default constants are well-defined.The default constants for proRatingConfig are correctly defined with sensible defaults (enabled: true, mode: 'prorate_prices').
11064-11080: LGTM! ProRatingConfig schema is well-structured.The proRatingConfig object schema is properly defined with:
- Clear field descriptions
- Appropriate enum values for mode
- Sensible default configuration
- Good use of nested describe() calls for comprehensive documentation
11118-11123: LGTM! Consistent default constants for update operations.The default constants for updatePlanBody proRatingConfig match the create operation defaults, ensuring consistency.
12091-12107: LGTM! Consistent proRatingConfig implementation for updates.The proRatingConfig schema for update operations is identical to the create operation, which is appropriate since this configuration should maintain the same structure and validation rules.
14855-14873: LGTM! Consistent proRatingConfig for subscription custom plans.The proRatingConfig implementation for subscription custom plans maintains consistency with the plan schemas, using the same structure and validation rules.
17342-17360: LGTM! Final proRatingConfig implementation is consistent.The last proRatingConfig implementation for changeSubscriptionBody customPlan maintains consistency with all other schemas, completing the comprehensive addition of pro-rating configuration across all relevant API endpoints.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
openmeter/ent/db/subscription_create.go (1)
316-324:⚠️ Potential issueMissing default value for billing_cadence field
Similar to the plan creation logic, the subscription's
billing_cadencefield lacks a default value whilepro_rating_confighas one. This inconsistency will cause subscription creation to fail if the billing cadence is not explicitly provided.Consider adding a default for
billing_cadence:if _, ok := _c.mutation.ProRatingConfig(); !ok { v := subscription.DefaultProRatingConfig() _c.mutation.SetProRatingConfig(v) } +if _, ok := _c.mutation.BillingCadence(); !ok { + v := subscription.DefaultBillingCadence() + _c.mutation.SetBillingCadence(v) +} if _, ok := _c.mutation.ID(); !ok { v := subscription.DefaultID() _c.mutation.SetID(v) }
♻️ Duplicate comments (20)
api/spec/package.json (1)
29-29: Invalid pnpm version in packageManager field.This issue was already identified in previous reviews - pnpm version 10.11.0 does not exist. The latest stable version is 10.6.5. Please update to a valid version.
openmeter/ent/db/runtime.go (2)
1168-1172: Initialization forpro_rating_configis correct, butbilling_cadenceis still unregistered onplan.
You’ve added thepro_rating_configdescriptors properly, but the adjacentbilling_cadencefield (atplanFields[2]) has no runtime setup. Without registeringDefaultBillingCadenceandValueScanner.BillingCadence, the builder will panic. Please add before thepro_rating_configlines:// planDescBillingCadence is the schema descriptor for billing_cadence field. planDescBillingCadence := planFields[2].Descriptor() // plan.DefaultBillingCadence holds the default value on creation for the billing_cadence field. plan.DefaultBillingCadence = planDescBillingCadence.Default.(func() string) // plan.ValueScanner.BillingCadence scans the billing_cadence field. plan.ValueScanner.BillingCadence = planDescBillingCadence.ValueScanner.(field.TypeValueScanner[string])
1344-1348: Initialization forpro_rating_configis correct, butbilling_cadenceis still unregistered onsubscription.
Similarly, after wiring uppro_rating_config, the newbilling_cadencefield (likely atsubscriptionFields[5]) must be initialized. Add:// subscriptionDescBillingCadence is the schema descriptor for billing_cadence field. subscriptionDescBillingCadence := subscriptionFields[5].Descriptor() // subscription.DefaultBillingCadence holds the default value on creation for the billing_cadence field. subscription.DefaultBillingCadence = subscriptionDescBillingCadence.Default.(func() string) // subscription.ValueScanner.BillingCadence scans the billing_cadence field. subscription.ValueScanner.BillingCadence = subscriptionDescBillingCadence.ValueScanner.(field.TypeValueScanner[string])openmeter/subscription/subscriptionspec.go (1)
314-347: The billing cadence alignment logic looks correct, but error handling needs attention.The method properly validates that billing cadences align by checking divisibility in both directions. However, it uses the
Comparemethod which ignores errors from the underlyingSubtractoperation.Run the following script to verify if the Compare method error handling has been addressed:
#!/bin/bash # Description: Check if the Compare method in pkg/isodate/date.go has been updated to handle errors # Check the Compare method implementation ast-grep --pattern 'func (p Period) Compare(p2 Period) $_'pkg/isodate/date.go (1)
121-125: Handle the error from Subtract operation.The error from
p.Period.Subtract(p2.Period)is being ignored, which could hide potential issues during period comparison.openmeter/productcatalog/plan.go (1)
120-173: Improve error handling and document comparison results.The validation logic has the same issues as previously identified in the past review comments.
api/client/javascript/src/zod/index.ts (4)
10188-10192: Add ISO8601 duration validation for billingCadence.The past review comment about adding ISO8601 duration validation hasn't been addressed. The field description mentions ISO8601 duration format but there's no validation to enforce this format.
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ),
11230-11234: Make billingCadence optional for update operations and add validation.Both past review comments remain unaddressed:
- billingCadence should be optional for update operations
- ISO8601 duration validation is missing
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ) + .optional(),
13937-13941: Add ISO8601 duration validation for subscription billingCadence.The same validation issue exists in subscription schemas. ISO8601 duration validation is missing.
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ),
16424-16428: Add ISO8601 duration validation for changeSubscription billingCadence.The same validation issue exists in the changeSubscription schema.
billingCadence: zod .string() + .regex(/^P(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$/, 'Must be a valid ISO8601 duration format') .describe( 'The default billing cadence for subscriptions using this plan.\nDefines how often customers are billed using ISO8601 duration format.\nExamples: \"P1M\" (monthly), \"P3M\" (quarterly), \"P1Y\" (annually).' ),api/openapi.yaml (6)
13063-13067: Breaking Change:billingCadenceadded to required list
MakingbillingCadencea required property in this schema is a backward-incompatible change for clients. Consider marking it optional or providing a server-side default to maintain compatibility.
19520-19524: Breaking Change:billingCadencerequired in plan version schema
Requiring this field may break existing workflows. Consider making it optional or supplying server-side defaults for backward compatibility.
19812-19816: Breaking Change:billingCadencerequired in plan identifier schema
Adding this required field can break client integrations. Consider making it optional or ensuring the server always populates it.
20002-20004: Breaking Change:billingCadencerequired in plan replacement object
Requiring this field in the replace/update payload is a breaking API change. Consider removing it from therequiredlist or providing a default value.
21385-21386: Breaking Change:billingCadencerequired in Subscription schema
Introducing this new mandatory field can break existing clients. Consider making it optional or ensuring all API responses include a default.
21860-21863: Breaking Change:billingCadencerequired in SubscriptionExpanded schema
This new mandatory field may break existing clients. Consider making it optional or providing server defaults.openmeter/ent/db/plan_create.go (1)
303-311: Missing default value for billing_cadence fieldThe
billing_cadencefield lacks a default value in thedefaults()method, whilepro_rating_confighas one. This will cause plan creation to fail if callers omit the billing cadence.openmeter/ent/db/subscription_create.go (1)
372-382: Validate subscription-plan billing cadence compatibilityThe validation checks for the presence and validity of the subscription's billing cadence but doesn't verify that it matches the associated plan's billing cadence. This could lead to subscriptions with incompatible cadences.
api/client/javascript/src/client/schemas.ts (2)
7954-7971: Verify this addresses the Plan schema from past review comments.This appears to be adding the missing fields to the
Planschema that was flagged in previous reviews. The implementation matches the pattern established in other schemas with consistent field definitions and documentation.
8166-8183: Required billingCadence could cause breaking changes for PlanCreate.While the implementation is correct, making
billingCadencerequired onPlanCreateinterface creates a breaking change for existing client code that constructs plan creation requests without this field.Let me verify if this breaking change affects existing client usage:
#!/bin/bash # Description: Check for plan creation calls that might be affected by required billingCadence # Expected: Identify usage patterns that need updating echo "=== Searching for PlanCreate usage patterns ===" rg -n "PlanCreate" --type ts api/client/javascript/ -A 3 -B 3 echo -e "\n=== Checking for plan creation method calls ===" rg -n "\.create\(" --type ts api/client/javascript/src/client/ -A 5 -B 5
🧹 Nitpick comments (13)
openmeter/productcatalog/plan_test.go (1)
33-37: Consider test coverage for different billing cadences and pro-rating configurations.While the current uniform values are appropriate for these specific tests, consider adding dedicated tests in the future to verify different billing cadences (e.g., weekly, quarterly) and pro-rating configurations (disabled, different modes) work correctly across the system.
Would you like me to help design test cases that cover different billing cadence and pro-rating scenarios?
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql (1)
1-102: Consider migration performance for large datasets.This migration performs complex queries that could be slow on large datasets due to multiple JOINs and window functions. Consider adding appropriate indexes or running during maintenance windows.
For production deployment, consider:
- Adding temporary indexes if needed:
CREATE INDEX CONCURRENTLY ...- Running during low-traffic periods
- Monitoring execution time and blocking behavior
The migration logic is correct, but performance should be validated against production-sized datasets.
pkg/isodate/date.go (1)
127-157: Excellent implementation of period divisibility check!The method thoroughly handles calendar variations by testing across different month lengths and ensures precision using decimal arithmetic. The requirement for whole hours makes sense for billing periods.
Consider making the error message more specific:
- return false, fmt.Errorf("divisible periods must be whole numbers of hours") + return false, fmt.Errorf("divisible periods must be whole numbers of hours (larger has %dm%ds, smaller has %dm%ds)", + l.Minutes(), l.Seconds(), s.Minutes(), s.Seconds())openmeter/productcatalog/plan.go (1)
262-264: Use consistent error wrapping format.For consistency with other error handling in this method (e.g., line 247), consider using
%winstead of%sfor proper error wrapping.- errs = append(errs, fmt.Errorf("invalid ProRatingConfig: %s", err)) + errs = append(errs, fmt.Errorf("invalid ProRatingConfig: %w", err))api/client/javascript/src/zod/index.ts (1)
10076-10081: Consider consolidating duplicate default constants.The same pro-rating configuration defaults are being defined multiple times across different schema sections (lines 10076-10081, 11118-11123, 13718-13725, 16237-16244). Consider defining these constants once and reusing them to improve maintainability.
+// Shared pro-rating configuration defaults +export const proRatingConfigEnabledDefault = true +export const proRatingConfigModeDefault = 'prorate_prices' +export const proRatingConfigDefault = { + enabled: true, + mode: 'prorate_prices', +} // Then reuse in each schema section: -export const createPlanBodyProRatingConfigEnabledDefault = true -export const createPlanBodyProRatingConfigModeDefault = 'prorate_prices' -export const createPlanBodyProRatingConfigDefault = { - enabled: true, - mode: 'prorate_prices', -} +export const createPlanBodyProRatingConfigDefault = proRatingConfigDefaultapi/openapi.yaml (7)
13097-13114: Add pattern constraint for ISO8601 duration
ThebillingCadenceproperty should strictly follow ISO8601 duration format. Consider adding apatternregex to enforce valid values, for example:pattern: '^P(?!$)(?:\d+Y)?(?:\d+M)?(?:\d+W)?(?:\d+D)?(?:T(?:\d+H)?(?:\d+M)?(?:\d+S)?)?$'This will catch invalid inputs early.
19598-19606: ExtractbillingCadenceinto a reusable component
The inline definition ofbillingCadenceis duplicated across multiple schemas. Extract it undercomponents/schemas/BillingCadenceand reference it via$refto reduce maintenance overhead.
19853-19861: Reuse or extractbillingCadencedefinition
This inline schema repeats the samebillingCadencestructure. Extract it to a shared component schema or reference an existing one via$refto keep the spec DRY.
20028-20036: DRY: Factor outbillingCadenceschema
This definition is duplicated here as well. ExtractbillingCadenceintocomponents/schemas/BillingCadenceand reference it to avoid copy-paste across plan-related schemas.
20265-20282: HardenProRatingConfigschema withadditionalProperties: false
AddingadditionalProperties: falsetoProRatingConfig(and potentiallyProRatingMode) will prevent extra unsupported fields and make the contract stricter:ProRatingConfig: type: object additionalProperties: false required: - enabled - mode # ...
21469-21478: Extract and validatebillingCadenceproperty
To reduce duplication, movebillingCadenceinto a shared component schema and reference it via$ref. Also consider apatternconstraint to enforce ISO8601 durations.
21941-21960: Consistent reuse ofbillingCadenceandProRatingConfig
ThebillingCadenceandproRatingConfigdefinitions are repeated. ExtractbillingCadenceinto a$ref’d component and continue to referenceProRatingConfigvia$refto keep schemas DRY.api/client/javascript/src/client/schemas.ts (1)
8433-8452: ProRatingMode enum seems incomplete with only one value.The
ProRatingModetype only defines'prorate_prices'as a literal type. This seems restrictive - typically billing systems support multiple pro-rating modes (e.g., full period, no pro-rating, etc.).Consider if additional pro-rating modes should be supported:
- ProRatingMode: 'prorate_prices' + ProRatingMode: 'prorate_prices' | 'full_period' | 'no_proration'However, if this matches the current backend implementation exactly, this may be intentional for this iteration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (66)
api/client/javascript/src/client/schemas.ts(8 hunks)api/client/javascript/src/zod/index.ts(12 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/package.json(1 hunks)api/spec/src/productcatalog/plan.tsp(2 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)e2e/productcatalog_test.go(6 hunks)openmeter/billing/worker/subscription/sync_test.go(7 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(16 hunks)openmeter/ent/db/plan.go(5 hunks)openmeter/ent/db/plan/plan.go(5 hunks)openmeter/ent/db/plan/where.go(3 hunks)openmeter/ent/db/plan_create.go(11 hunks)openmeter/ent/db/plan_update.go(7 hunks)openmeter/ent/db/predicate/predicate.go(2 hunks)openmeter/ent/db/runtime.go(2 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(5 hunks)openmeter/ent/db/subscription/where.go(3 hunks)openmeter/ent/db/subscription_create.go(12 hunks)openmeter/ent/db/subscription_update.go(7 hunks)openmeter/ent/schema/productcatalog.go(3 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/adapter/feature_test.go(1 hunks)openmeter/productcatalog/addon/adapter/mapping.go(2 hunks)openmeter/productcatalog/errors.go(2 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/plan/adapter/mapping.go(2 hunks)openmeter/productcatalog/plan/adapter/plan.go(2 hunks)openmeter/productcatalog/plan/httpdriver/mapping.go(6 hunks)openmeter/productcatalog/plan/serializer_test.go(1 hunks)openmeter/productcatalog/plan/service.go(3 hunks)openmeter/productcatalog/plan/service/plan.go(1 hunks)openmeter/productcatalog/plan/service/service_test.go(1 hunks)openmeter/productcatalog/plan_test.go(13 hunks)openmeter/productcatalog/planaddon_test.go(3 hunks)openmeter/productcatalog/pro_rating.go(1 hunks)openmeter/productcatalog/subscription/http/mapping.go(4 hunks)openmeter/productcatalog/subscription/plan.go(1 hunks)openmeter/productcatalog/subscription/service/plan.go(1 hunks)openmeter/productcatalog/testutils/plan.go(2 hunks)openmeter/subscription/addon/diff/restore_test.go(2 hunks)openmeter/subscription/aligment_test.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(1 hunks)openmeter/subscription/repository.go(1 hunks)openmeter/subscription/subscription.go(2 hunks)openmeter/subscription/subscriptionspec.go(3 hunks)openmeter/subscription/subscriptionview.go(1 hunks)openmeter/subscription/testutils/plan.go(1 hunks)openmeter/subscription/workflow/service/addon_test.go(2 hunks)openmeter/subscription/workflow/service/subscription.go(1 hunks)openmeter/subscription/workflow/service/subscription_test.go(2 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)test/billing/subscription_test.go(2 hunks)test/customer/customer.go(3 hunks)test/subscription/scenario_editaligned_test.go(3 hunks)test/subscription/scenario_editcancel_test.go(2 hunks)tools/migrate/entitlement_test.go(5 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql(1 hunks)tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql(1 hunks)tools/migrate/productcatalog_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (27)
openmeter/productcatalog/subscription/service/plan.go (2)
pkg/models/errors.go (1)
ErrorWithFieldPrefix(340-346)pkg/models/fieldselector.go (2)
NewFieldSelectors(229-231)NewFieldSelector(177-179)
openmeter/subscription/repo/subscriptionrepo.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)api/api.gen.go (1)
ProRatingConfig(5852-5858)api/client/go/client.gen.go (1)
ProRatingConfig(5382-5388)
openmeter/productcatalog/plan/service/plan.go (2)
openmeter/productcatalog/alignment.go (1)
Alignment(3-6)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/subscription/workflow/service/addon_test.go (4)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)pkg/isodate/date.go (1)
MustParse(196-203)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/testutils/plan.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(196-203)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/subscription.go (6)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)pkg/isodate/date.go (1)
Period(48-50)api/client/javascript/src/client/schemas.ts (2)
Period(10888-10888)ProRatingConfig(10915-10915)api/api.gen.go (2)
Period(5465-5471)ProRatingConfig(5852-5858)
test/customer/customer.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(196-203)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
test/subscription/scenario_editcancel_test.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(196-203)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/subscription/workflow/service/subscription_test.go (3)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
MustParse(196-203)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/productcatalog/plan/service/service_test.go (1)
pkg/isodate/date.go (1)
MustParse(196-203)
openmeter/ent/schema/subscription.go (4)
pkg/isodate/date.go (1)
String(17-17)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)openmeter/ent/schema/productcatalog.go (1)
ProRatingConfigValueScanner(200-200)
openmeter/ent/schema/productcatalog.go (4)
pkg/isodate/date.go (1)
String(17-17)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)openmeter/ent/db/plan/plan.go (1)
ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (1)
ValueScanner(152-154)
openmeter/ent/db/subscription.go (6)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)pkg/isodate/date.go (1)
String(17-17)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/subscription/subscription.go (10)
FieldID(18-18)FieldNamespace(20-20)FieldName(36-36)FieldDescription(38-38)FieldPlanID(40-40)FieldCustomerID(42-42)FieldCurrency(44-44)FieldBillingCadence(46-46)FieldProRatingConfig(48-48)ValueScanner(152-154)
openmeter/ent/db/runtime.go (4)
openmeter/ent/db/plan/plan.go (2)
DefaultProRatingConfig(130-130)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (2)
DefaultProRatingConfig(148-148)ValueScanner(152-154)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
openmeter/productcatalog/pro_rating.go (4)
api/api.gen.go (3)
ProRatingMode(5861-5861)ProRatingModeProratePrices(700-700)ProRatingConfig(5852-5858)api/client/go/client.gen.go (3)
ProRatingMode(5391-5391)ProRatingModeProratePrices(637-637)ProRatingConfig(5382-5388)pkg/models/validator.go (1)
Validate(16-26)pkg/models/errors.go (1)
NewNillableGenericValidationError(129-135)
openmeter/productcatalog/plan/service.go (2)
pkg/isodate/date.go (1)
Period(48-50)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)
e2e/productcatalog_test.go (6)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)openmeter/ent/db/subscriptionitem/where.go (1)
BillingCadence(147-150)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
openmeter/ent/db/plan/plan.go (7)
openmeter/ent/db/subscription/subscription.go (5)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)ByBillingCadence(226-228)OrderOption(158-158)ByProRatingConfig(231-233)openmeter/ent/db/planratecard/planratecard.go (3)
FieldBillingCadence(45-45)ByBillingCadence(206-208)OrderOption(143-143)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/billinginvoiceusagebasedlineconfig/billinginvoiceusagebasedlineconfig.go (1)
OrderOption(80-80)openmeter/ent/db/addon/addon.go (1)
OrderOption(146-146)openmeter/ent/db/planaddon/planaddon.go (1)
OrderOption(102-102)openmeter/ent/db/planphase/planphase.go (1)
OrderOption(105-105)
openmeter/productcatalog/errors.go (2)
pkg/models/errors.go (1)
ErrorCode(294-294)pkg/models/validationissue.go (4)
NewValidationIssue(219-230)WithFieldString(196-202)WithCriticalSeverity(210-212)WithWarningSeverity(214-216)
openmeter/ent/db/subscription/subscription.go (7)
openmeter/ent/db/plan/plan.go (5)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)ByBillingCadence(198-200)OrderOption(140-140)ByProRatingConfig(203-205)openmeter/ent/db/subscriptionitem/subscriptionitem.go (3)
FieldBillingCadence(58-58)ByBillingCadence(257-259)OrderOption(159-159)api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
OrderOption(285-285)openmeter/ent/db/entitlement/entitlement.go (1)
OrderOption(195-195)openmeter/ent/db/subscriptionphase/subscriptionphase.go (1)
OrderOption(113-113)
openmeter/productcatalog/adapter/feature_test.go (4)
api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10915-10915)api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingModeProratePrices(700-700)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingModeProratePrices(637-637)
openmeter/ent/db/plan.go (5)
openmeter/ent/db/plan/where.go (1)
BillingCadence(120-123)pkg/isodate/date.go (1)
String(17-17)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/plan.go (14)
FieldID(18-18)FieldNamespace(20-20)FieldName(30-30)FieldDescription(32-32)FieldKey(34-34)FieldCurrency(40-40)FieldBillingCadence(42-42)FieldCreatedAt(24-24)FieldUpdatedAt(26-26)FieldDeletedAt(28-28)FieldEffectiveFrom(46-46)FieldEffectiveTo(48-48)FieldProRatingConfig(44-44)ValueScanner(134-136)openmeter/ent/db/subscription/subscription.go (11)
FieldID(18-18)FieldNamespace(20-20)FieldName(36-36)FieldDescription(38-38)FieldCurrency(44-44)FieldBillingCadence(46-46)FieldCreatedAt(22-22)FieldUpdatedAt(24-24)FieldDeletedAt(26-26)FieldProRatingConfig(48-48)ValueScanner(152-154)
openmeter/ent/db/migrate/schema.go (2)
openmeter/ent/db/plan/plan.go (1)
Columns(81-98)openmeter/ent/db/subscription/subscription.go (1)
Columns(99-116)
openmeter/subscription/workflow/service/subscription.go (1)
pkg/models/errors.go (1)
NewGenericValidationError(138-140)
openmeter/ent/db/plan/where.go (5)
openmeter/ent/db/subscription/where.go (1)
BillingCadence(132-135)openmeter/ent/db/planratecard/where.go (1)
BillingCadence(111-114)pkg/isodate/date.go (1)
String(17-17)openmeter/ent/db/plan.go (2)
Plan(19-57)Plan(100-121)openmeter/ent/db/plan/plan.go (1)
FieldBillingCadence(42-42)
api/client/javascript/src/client/schemas.ts (3)
api/api.gen.go (2)
ProRatingConfig(5852-5858)ProRatingMode(5861-5861)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingMode(14-14)api/client/go/client.gen.go (2)
ProRatingConfig(5382-5388)ProRatingMode(5391-5391)
openmeter/ent/db/mutation.go (4)
pkg/isodate/date.go (1)
String(17-17)openmeter/productcatalog/pro_rating.go (1)
ProRatingConfig(23-29)openmeter/ent/db/plan/plan.go (2)
FieldBillingCadence(42-42)FieldProRatingConfig(44-44)openmeter/ent/db/subscription/subscription.go (2)
FieldBillingCadence(46-46)FieldProRatingConfig(48-48)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: CI
- GitHub Check: E2E
- GitHub Check: Analyze (go)
🔇 Additional comments (160)
tools/migrate/entitlement_test.go (1)
21-122: LGTM! Excellent code style improvement.The explicit naming of the
stopsfield in therunnerstruct initialization enhances code readability and maintainability. This is a good practice that makes the code more self-documenting and reduces ambiguity.Also applies to: 138-474, 495-904
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sql (1)
1-4: LGTM! Correctly implemented down migration.The rollback script properly removes the
pro_rating_configandbilling_cadencecolumns from both theplansandsubscriptionstables. The SQL syntax is correct and the operations are appropriate for reversing the billing cadence and pro-rating feature additions.openmeter/subscription/subscription.go (2)
9-9: LGTM! Appropriate import for new functionality.The
isodatepackage import is correctly added to support the newBillingCadencefield.
30-31: LGTM! Well-defined fields for billing cadence and pro-rating.The new fields are properly implemented:
BillingCadenceusesisodate.Periodfor ISO8601 duration representationProRatingConfiguses the appropriateproductcatalog.ProRatingConfigtype- JSON tags follow consistent snake_case naming convention
I note from the conversation history that the
AsEntityInputmethod issue was already resolved in PR #2922.openmeter/productcatalog/subscription/service/plan.go (1)
63-64: LGTM! Improved error handling with field context.This change enhances error handling by providing structured validation errors that include the "plan" field context, making it easier to identify which field validation failed. This is a good improvement over generic wrapped errors.
openmeter/productcatalog/plan/serializer_test.go (1)
37-41: LGTM! Proper test coverage for new fields.The test correctly includes the new
BillingCadenceandProRatingConfigfields with reasonable values. This ensures the serialization/deserialization functionality works correctly for the new billing cadence and pro-rating configuration features.openmeter/subscription/subscriptionview.go (1)
241-246: LGTM! Correct field propagation in subscription view.The
CreateSubscriptionPlanInputstruct is properly expanded to include the newBillingCadenceandProRatingConfigfields from the subscription object. This ensures these fields are correctly propagated when creating subscription views, which is essential for the billing cadence and pro-rating functionality.openmeter/subscription/addon/diff/restore_test.go (2)
23-23: LGTM! Required import for new field support.The
isodatepackage import is correctly added to support theisodate.MustParsefunction used for setting theBillingCadencefield in the test data.
354-358: LGTM! Consistent test data with new fields.The test data is properly updated to include the new
BillingCadenceandProRatingConfigfields with values consistent with other tests across the codebase. This ensures the addon diff restore functionality is tested with the complete data model including the new billing cadence and pro-rating features.openmeter/productcatalog/plan/service/plan.go (1)
754-762: LGTM! Proper inheritance of billing configuration fields.The addition of
Alignment,BillingCadence, andProRatingConfigto the copied metadata ensures that new plan versions inherit the billing configuration from their source plan. This is consistent with the existing pattern of copying plan metadata and aligns with the broader feature implementation.openmeter/productcatalog/subscription/plan.go (1)
74-77: LGTM! Proper propagation of billing configuration to subscription plan input.The addition of
BillingCadenceandProRatingConfigfields ensures that billing configuration from the plan is correctly propagated to the subscription plan input. This maintains consistency between plan definitions and subscription creation.openmeter/productcatalog/adapter/feature_test.go (1)
322-326: LGTM! Test setup properly includes new billing configuration fields.The test correctly sets up the plan with:
BillingCadenceset to "P1M" (one month period)ProRatingConfigwith enabled pro-rating using theprorate_pricesmodeThese values are consistent with the domain model definitions and ensure the test plan reflects the extended billing model structure.
openmeter/subscription/repository.go (1)
30-34: LGTM! Proper addition of billing configuration fields to subscription input.The new fields are correctly added with:
- Appropriate types (
isodate.Periodfor billing cadence,productcatalog.ProRatingConfigfor pro-rating)- Consistent JSON tag naming convention (
billing_cadence,pro_rating_config)- Clear documentation comments explaining their purpose
This extends the subscription creation input to support the new billing configuration features.
openmeter/subscription/workflow/service/subscription_test.go (2)
28-28: LGTM: Import addition for isodate packageThe import is correctly added to support the
MustParsefunction used in the test plan configuration.
867-871: LGTM: Test plan enhanced with billing cadence and pro-rating configurationThe test plan
examplePlanInput2is correctly updated to include the new requiredBillingCadencefield and optionalProRatingConfigfield. The values are appropriate:
BillingCadence: isodate.MustParse(t, "P1M")sets a one-month billing cycleProRatingConfigenables pro-rating withProRatingModeProratePricesmodeThis change aligns the test data with the system-wide schema enhancements for explicit billing cadence and pro-rating support.
openmeter/subscription/aligment_test.go (1)
36-40: LGTM: Plan test data updated with billing cadence and pro-rating configurationThe test plan metadata is correctly enhanced with:
BillingCadence: isodate.MustParse(t, "P1M")for monthly billing cyclesProRatingConfigwith enabled pro-rating using price proration modeThis ensures the aligned billing period calculation tests work correctly with the enhanced plan schema that now includes explicit billing cadence and pro-rating configuration.
test/subscription/scenario_editaligned_test.go (3)
23-23: LGTM: Import addition for isodate packageThe import is correctly added to support the
MustParsefunction used in test plan configurations.
60-64: LGTM: Aligned subscription test plan enhanced with billing configurationThe test plan for
TestEditingEntitlementOfAlignedSubis correctly updated with:
BillingCadence: isodate.MustParse(t, "P1M")for monthly billingProRatingConfigwith enabled pro-rating using price proration modeThis ensures the entitlement editing test works with the enhanced plan schema.
241-245: LGTM: Unaligned subscription test plan enhanced with billing configurationThe test plan for
TestEditingEntitlementOfUnalignedSubis consistently updated with the same billing cadence and pro-rating configuration as the aligned test. This maintains consistency across both aligned and unaligned subscription test scenarios.test/customer/customer.go (3)
22-22: LGTM: Import addition for isodate packageThe import is correctly added to support the
MustParsefunction used in the example plan configurations.
228-232: LGTM: Example plan enhanced for customer update testThe
emptyExamplePlanused inTestUpdateWithSubscriptionPresentis correctly updated with:
BillingCadence: isodate.MustParse(t, "P1M")for monthly billingProRatingConfigwith enabled pro-rating using price proration modeThis ensures the customer update test works correctly when a subscription is present with the enhanced plan schema.
563-567: LGTM: Example plan enhanced for customer deletion testThe
emptyExamplePlanused inTestDeleteis consistently updated with the same billing cadence and pro-rating configuration. This ensures the customer deletion test works correctly when active subscriptions exist with the enhanced plan schema.openmeter/productcatalog/planaddon_test.go (1)
46-54: LGTM! Test data properly updated for new billing cadence and pro-rating fields.The test cases correctly include the new
BillingCadenceandProRatingConfigfields with appropriate values:
BillingCadenceset toP1M(one-month ISO8601 duration)ProRatingConfigenabled withProRatingModeProratePricesmodeThis ensures the validation scenarios test complete plan metadata structures aligned with the new billing model.
Also applies to: 214-222, 409-417
openmeter/subscription/repo/subscriptionrepo.go (1)
106-107: LGTM! Proper field persistence implementation.The repository correctly handles the new billing cadence and pro-rating configuration fields:
BillingCadenceis properly converted to ISO string format using.ISOString()for database storageProRatingConfigis set directly as a structured fieldThis follows the established pattern for field persistence in the subscription creation flow.
openmeter/subscription/testutils/plan.go (1)
59-67: LGTM! Test utility properly enhanced with new billing fields.The
BuildTestPlanfunction now includes sensible defaults for the new billing cadence and pro-rating configuration:
BillingCadenceset toP1M(monthly billing cycle)ProRatingConfigenabled with prorate prices modeThese defaults ensure test plans are created with complete metadata, supporting comprehensive testing of the new billing features.
test/subscription/scenario_editcancel_test.go (2)
24-24: LGTM! Required import added for ISO date handling.The
isodateimport is properly added to support parsing of billing cadence duration strings in the test.
58-65: LGTM! Integration test enhanced with complete billing configuration.The test plan creation now includes the new billing cadence and pro-rating configuration fields with appropriate values:
- Monthly billing cadence (
P1M)- Pro-rating enabled with price proration mode
This ensures the integration test scenarios properly cover subscription editing and cancellation workflows with the complete billing model.
test/billing/subscription_test.go (2)
115-123: LGTM: Test data properly updated with new billing cadence and pro-rating fields.The test correctly adds the new
BillingCadenceandProRatingConfigfields to the paid plan test data. The use ofisodate.MustParse(s.T(), "P1D")for a one-day billing cadence is appropriate for testing, and the pro-rating configuration withEnabled: trueandProRatingModeProratePricesfollows the expected pattern.
166-174: LGTM: Consistent test data configuration for free plan.The free plan test data is correctly updated with the same billing cadence and pro-rating configuration as the paid plan, ensuring consistency across the test suite.
openmeter/productcatalog/plan/adapter/mapping.go (2)
17-21: LGTM: Proper billing cadence parsing with error handling.The billing cadence parsing correctly uses the
Parse()method and provides appropriate error handling with a descriptive error message that includes the invalid billing cadence value for debugging.
39-40: LGTM: New fields correctly included in plan metadata.The parsed
BillingCadenceandProRatingConfigfields are properly assigned to thePlanMetastruct, ensuring the domain model includes the new billing configuration.openmeter/subscription/workflow/service/subscription.go (1)
144-151: LGTM: Essential billing cadence alignment validation added.The validation correctly ensures billing cadences are aligned before updating a subscription, which is crucial for maintaining billing consistency. The error handling follows the established pattern using
models.NewGenericValidationError, and the placement after applying customizations but before the database update is appropriate.openmeter/subscription/repo/mapping.go (2)
31-34: LGTM: Consistent billing cadence parsing implementation.The billing cadence parsing follows the same pattern as the plan adapter, using the
Parse()method with proper error handling and a descriptive error message for debugging failed parsing attempts.
61-62: LGTM: New subscription fields correctly mapped.The parsed
BillingCadenceandProRatingConfigfields are properly assigned to the subscription struct, ensuring the domain model includes the new billing configuration data from the database.openmeter/productcatalog/testutils/plan.go (2)
11-11: LGTM: Import addition looks good.The import of the
isodatepackage is necessary for parsing the billing cadence duration string.
29-33: LGTM: New billing cadence and pro-rating fields properly initialized.The addition of
BillingCadenceandProRatingConfigfields aligns with the PR objectives. The default values are appropriate for test scenarios:
P1M(one month) is a common billing cadence- Pro-rating enabled with price pro-rating mode is a reasonable default for tests
Using
isodate.MustParse(t, "P1M")is appropriate in test utilities as it will fail fast on invalid duration strings.openmeter/ent/db/predicate/predicate.go (2)
215-224: LGTM: Generated predicate function follows established pattern.The
PlanOrErrfunction correctly implements the error-aware predicate pattern consistent with other*OrErrfunctions in this generated file. The implementation properly checks for errors and adds them to the SQL selector before early return.
249-258: LGTM: Generated predicate function follows established pattern.The
SubscriptionOrErrfunction correctly implements the error-aware predicate pattern consistent with other*OrErrfunctions in this generated file. The implementation is identical to the pattern used throughout the file.openmeter/productcatalog/plan_test.go (3)
13-13: LGTM: Import addition necessary for billing cadence parsing.The import of the
isodatepackage is required for parsing ISO 8601 duration strings used in the billing cadence field.
33-37: LGTM: Consistent initialization of new fields across test cases.All test cases properly initialize the new
BillingCadenceandProRatingConfigfields with consistent values. The choice ofP1M(one month) and enabled pro-rating with price pro-rating mode provides good default values for these status-focused tests.Also applies to: 50-54, 67-71, 84-88, 101-105, 118-122, 135-139, 152-156, 169-173
197-203: LGTM: Alignment enforcement tests properly include new fields.The alignment enforcement tests correctly include the new billing cadence and pro-rating configuration fields, maintaining test consistency while preserving the original test logic for billing alignment validation.
Also applies to: 252-258, 307-313
openmeter/productcatalog/plan/adapter/plan.go (2)
183-184: LGTM! Proper handling of new fields in plan creation.The implementation correctly adds the new
BillingCadenceandProRatingConfigfields to plan creation using appropriate conversion methods.
428-430: LGTM! Correct nullable setters for plan updates.The update operation properly uses nullable setters (
SetNillableBillingCadenceandSetNillableProRatingConfig) and the appropriate conversion methodISOStringPtrOrNil()to handle optional values in updates.openmeter/productcatalog/addon/adapter/mapping.go (2)
187-190: LGTM! Proper parsing and error handling for billing cadence.The implementation correctly parses the billing cadence from the database using the
Parse()method and provides a descriptive error message that includes the invalid value for debugging purposes.
207-208: LGTM! Correct assignment of new fields to plan metadata.The parsed billing cadence and pro-rating configuration are properly assigned to the
PlanMetastruct, maintaining consistency with the domain model structure.api/spec/src/productcatalog/subscription.tsp (2)
95-104: LGTM! Well-documented billing cadence field specification.The
billingCadencefield is properly defined with:
- Appropriate visibility and summary annotations
- Correct ISO8601 duration encoding
- Clear example demonstrating monthly billing
- Read-only visibility which makes sense for subscription metadata
106-114: LGTM! Proper pro-rating configuration with sensible defaults.The
proRatingConfigfield is well-specified with:
- Optional nature with reasonable default values
- Enabled by default with
proratePricesmode- Appropriate visibility and summary annotations
- Consistent with the domain model structure
openmeter/ent/schema/subscription.go (2)
40-42: LGTM! Proper billing cadence field definition.The
billing_cadencefield is correctly defined using theisodate.StringGo type with an appropriate comment describing its purpose as the default billing cadence for subscriptions.
43-55: LGTM! Comprehensive pro-rating configuration field definition.The
pro_rating_configfield is well-implemented with:
- Correct Go type
productcatalog.ProRatingConfig- Proper JSON storage configuration for PostgreSQL
- Appropriate default values (enabled with
proratePricesmode)- Consistent value scanner usage pattern
- Clear documentation comment
The default configuration aligns with the API specification and provides sensible behavior out of the box.
openmeter/subscription/workflow/service/addon_test.go (2)
27-27: LGTM: Import addition for isodate supportThe import addition supports the new billing cadence functionality in the test.
571-575: LGTM: Test enhancement with billing cadence and pro-rating configurationThe test case correctly adds the new billing cadence (
P1Mfor monthly) and pro-rating configuration (enabled withprorate_pricesmode) to ensure that addon quantity changes are tested with these new features. This provides good test coverage for the billing cadence and pro-rating functionality when modifying addon quantities.api/spec/src/productcatalog/prorating.tsp (1)
1-31: LGTM: Well-designed pro-rating configuration schemaThe TypeSpec schema is well-structured with:
- Clear namespace organization under
OpenMeter.ProductCatalog- Appropriate union type for
ProRatingModewith initialprorate_pricesoption- Sensible defaults (
enabled: true,mode: ProRatingMode.proratePrices)- Good documentation with summaries and comments
- Extensible design allowing for future pro-rating modes
The schema provides a solid foundation for the pro-rating configuration feature.
openmeter/ent/db/subscription.go (3)
16-16: LGTM: Proper imports for new field typesThe imports for
productcatalogandisodatepackages are correctly added to support the newBillingCadenceandProRatingConfigfields.Also applies to: 18-18
52-55: LGTM: New fields correctly added to Subscription structThe
BillingCadence(asisodate.String) andProRatingConfig(asproductcatalog.ProRatingConfig) fields are properly defined with appropriate JSON tags and documentation comments.
137-137: LGTM: Comprehensive implementation of new field handlingThe generated code correctly implements:
- Scanning support for both fields in
scanValuesmethod- Proper value assignment with type conversion in
assignValuesmethod- String representation inclusion for debugging
- Custom value scanner usage for
ProRatingConfigJSON handlingThe implementation follows established patterns and handles the different data types appropriately.
Also applies to: 141-142, 248-253, 254-259, 367-372
openmeter/productcatalog/plan/service/service_test.go (1)
28-30: LGTM: Improved consistency with ISO8601 duration formatThe refactor from
isodate.FromDurationtoisodate.MustParsewith ISO8601 duration strings ("P1M","P2M","P6M") improves consistency with how billing cadence is represented throughout the codebase. UsingMustParsewith the testing context is appropriate as it will fail the test immediately if parsing fails, providing clear error messages.openmeter/productcatalog/plan/service.go (4)
13-13: LGTM: Import addition is appropriate.The import of
pkg/isodateis necessary for the newBillingCadencefield and follows Go import conventions.
155-160: LGTM: New fields are well-designed.The addition of
BillingCadenceandProRatingConfigfields toUpdatePlanInputis properly implemented:
- Both fields are optional (pointer types) which is appropriate for update operations
- JSON tags are correctly specified with
omitemptyfor optional fields- Field comments clearly describe their purpose
- Types align with the system's billing cadence and pro-rating architecture
199-206: LGTM: Equal method implementation is correct.The equality checks for the new fields are properly implemented:
- Null-safety: Both checks verify the input field is not nil before comparison
BillingCadenceuses string representation comparison, which is appropriate for ISO8601 periodsProRatingConfiguses the type's ownEqualmethod, following good OOP practices
155-160:❓ Verification inconclusive
Consider adding validation for the new fields.
While the new fields are properly defined, I notice that the
Validate()method (lines 222-261) doesn't include validation logic forBillingCadenceandProRatingConfig. Consider whether validation should be added here or verify that it's handled elsewhere in the system.
🏁 Script executed:
#!/bin/bash # Description: Check if validation for BillingCadence and ProRatingConfig is handled elsewhere # Expected: Find validation logic for these fields in related files echo "Searching for BillingCadence validation..." rg -A 5 -B 5 "BillingCadence.*[Vv]alidat" --type go echo "Searching for ProRatingConfig validation..." rg -A 5 -B 5 "ProRatingConfig.*[Vv]alidat" --type go echo "Searching for validation in productcatalog package..." rg -A 5 "func.*Validate.*" openmeter/productcatalog/ --type go | grep -E "(BillingCadence|ProRating)"Length of output: 11849
To confirm whether the service‐level
Validate()covers these fields, let’s inspectopenmeter/productcatalog/plan/service.go:
🏁 Script executed:
#!/bin/bash echo "1. Locate Validate method in service.go" rg -n "func .*Validate" openmeter/productcatalog/plan/service.go echo echo "2. Show its surrounding context" rg -A10 -B5 "func .*Validate" openmeter/productcatalog/plan/service.go echo echo "3. Check for BillingCadence and ProRatingConfig inside that method" rg -A5 -B5 "BillingCadence" openmeter/productcatalog/plan/service.go rg -A5 -B5 "ProRatingConfig" openmeter/productcatalog/plan/service.goLength of output: 5061
Let’s inspect the
UpdatePlanservice implementation to see if it applies domain-level validation after merging these fields:
🏁 Script executed:
#!/bin/bash echo "1. Locate UpdatePlan method signature" rg -n "func .*UpdatePlan" openmeter/productcatalog/plan/service.go echo echo "2. Show its surrounding context (including validation calls)" rg -A20 -B5 "func .*UpdatePlan" openmeter/productcatalog/plan/service.goLength of output: 1412
To verify if the service’s UpdatePlan path invokes domain‐level validation (and thus covers BillingCadence/ProRatingConfig), let’s inspect the top of
service.gofor the service definition:
🏁 Script executed:
#!/bin/bash echo "📄 Showing file header and service struct:" head -n 50 openmeter/productcatalog/plan/service.goLength of output: 1588
Let’s verify how the
UpdatePlanoperation applies validation on the mergedPlanobject:
🏁 Script executed:
#!/bin/bash # 1. Locate the service implementation struct echo "🔎 Looking for the service struct definition:" rg -n "type service" -n -A5 openmeter/productcatalog/plan/service.go echo # 2. Find the UpdatePlan implementation on that struct echo "🔎 Showing the UpdatePlan method body:" rg -A20 -B5 "func (s \\*service) UpdatePlan" openmeter/productcatalog/plan/service.goLength of output: 353
Ensure BillingCadence & ProRatingConfig Are Validated on Update
I noticed that while your domain-level
Plan.Validate()(inopenmeter/productcatalog/plan.go) covers bothBillingCadenceandProRatingConfig, the update path skips it. In particular:
- In
openmeter/productcatalog/plan/service.go,
func (i UpdatePlanInput) Validate() error(≈ lines 222–261) does not check the new fields.- I couldn’t find a call to
Plan.Validate()in theUpdatePlanservice implementation to catch invalid values after merging.Please verify that the update flow enforces validation for these fields. You can either:
• Call
Plan.Validate()on the merged entity in
func (s *service) UpdatePlan(...){…}
• Or add explicit checks inUpdatePlanInput.Validate(), e.g.:```diff if i.EffectiveFrom != nil || i.EffectiveTo != nil { if err := i.EffectivePeriod.Validate(); err != nil { errs = append(errs, fmt.Errorf("invalid effective period: %w", err)) } } + if i.BillingCadence != nil { + if i.BillingCadence.IsZero() { + errs = append(errs, fmt.Errorf("invalid billingCadence: must not be empty")) + } + } + if i.ProRatingConfig != nil { + if err := i.ProRatingConfig.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid proRatingConfig: %w", err)) + } + } ```Verify that one of these approaches is in place before saving the updated plan.
openmeter/billing/worker/subscription/sync_test.go (5)
157-165: Include billing cadence and pro-rating config in HappyPath plan setupThe
TestSubscriptionHappyPathdefinition is correctly extended withBillingCadenceandProRatingConfigto reflect the new model fields. This ensures the test mirrors production behavior.
575-582: Add billing cadence and pro-rating config in InArrearsProrating testThe plan metadata in
TestInArrearsProratingnow includesBillingCadenceandProRatingConfig, matching the domain changes. The test will validate the new configuration paths.
1391-1395: Extend DefactoZeroPrices test with new plan fieldsThe
TestDefactoZeroPricesplan specification has been enhanced to includeBillingCadenceandProRatingConfig, aligning it with updated plan metadata requirements.
1467-1471: Update AlignedSubscriptionInvoicing with new PlanMeta fieldsIn
TestAlignedSubscriptionInvoicing, the initial plan setup is updated to includeBillingCadenceandProRatingConfig, ensuring consistency in alignment tests.
3806-3814: Propagate new fields in helper createSubscriptionFromPlanPhasesThe helper
createSubscriptionFromPlanPhasescorrectly injectsBillingCadenceandProRatingConfiginto created plans, centralizing the test setup for all phased subscription tests.openmeter/ent/db/plan.go (5)
14-15: LGTM! Proper import additions for new field types.The imports for
productcatalogandisodatepackages are correctly added to support the newBillingCadenceandProRatingConfigfields.
45-48: LGTM! Well-documented field additions.The new fields are properly typed and documented:
BillingCadenceusesisodate.Stringfor ISO8601 duration handlingProRatingConfiguses the domain-specific type from productcatalog packageThe comments clearly explain the purpose of each field.
110-115: LGTM! Correct handling in scanValues method.The new fields are properly integrated into the scanning logic:
BillingCadenceis handled as a string field (appropriate for isodate.String)ProRatingConfiguses the custom value scanner, which is the correct approach for complex types in ent
207-218: LGTM! Proper field assignment logic.The assignment logic correctly handles both new fields:
BillingCadenceis cast from string toisodate.StringProRatingConfiguses the value scanner'sFromValuemethod with proper error handling
321-326: LGTM! String method updated consistently.The
String()method is properly updated to include the new fields in the debug output, maintaining consistency with the existing format.pkg/isodate/date_test.go (1)
81-480: Excellent comprehensive test coverage for DivisibleBy method.This test function provides thorough coverage of the
DivisibleBymethod with well-structured test cases:
- Compatible periods: Properly tests various divisible combinations (yearly, monthly, weekly, daily, hourly)
- Incompatible periods: Tests cases where periods don't divide evenly
- Edge cases: Handles zero periods and boundary conditions
- Error validation: Tests rejection of periods with minutes/seconds
- Mixed units: Tests cross-unit divisibility (weeks/days, hours/days)
The test cases are clearly named and cover the business logic requirements for billing cadence alignment validation. This robust testing will help ensure reliable billing cadence compatibility checks throughout the system.
openmeter/productcatalog/errors.go (2)
392-399: LGTM! Appropriate critical error for billing cadence validation.The error definition correctly enforces the business rule that billing cadence must be at least 1 month. Using critical severity is appropriate since this represents a fundamental constraint violation that would prevent proper billing operations.
472-479: LGTM! Well-defined compatibility warning.The error definition appropriately flags billing cadence compatibility issues between plans and rate cards as a warning. This allows the system to highlight potential misalignments while still permitting the configuration, which provides flexibility for complex billing scenarios.
api/spec/src/productcatalog/plan.tsp (3)
4-4: LGTM! Necessary import for pro-rating types.The import of
./prorating.tspis correctly added to support the newProRatingConfigtype used in the Plan model.
83-92: Well-designed billing cadence property.The
billingCadenceproperty is properly defined with:
- Appropriate visibility for read/create/update operations
- ISO8601 duration encoding for standardized format
- Clear documentation and practical example ("P1M" for monthly billing)
- Descriptive summary and comprehensive description
This provides a solid foundation for billing frequency configuration across the API.
94-102: Thoughtful pro-rating configuration with sensible defaults.The
proRatingConfigproperty is well-implemented:
- Optional configuration allows flexibility
- Sensible default enables pro-rating with
proratePricesmode- Proper visibility annotations match other configurable properties
- Clear documentation explains the purpose
The default configuration strikes a good balance between enabling pro-rating capabilities while allowing customization when needed.
openmeter/productcatalog/pro_rating.go (4)
10-20: LGTM! Well-designed enum-like type with extensibility in mind.The constant naming follows Go conventions, and the
Values()method provides a clean way to enumerate all possible pro-rating modes. This design will make it easy to add new modes in the future.
22-29: LGTM! Clean struct design with proper documentation.The struct fields are well-named and documented, with appropriate JSON tags for API serialization.
31-43: LGTM! Solid validation implementation.The validation logic properly checks for valid pro-rating modes and uses the established error handling patterns from the models package. The switch statement approach makes it easy to add validation for new modes.
45-48: LGTM! Correct equality comparison implementation.The method properly compares both struct fields and will be useful for testing and change detection scenarios.
e2e/productcatalog_test.go (6)
211-215: LGTM! Appropriate test data for billing cadence.The addition of
BillingCadence: "P1M"properly tests monthly billing scenarios using the correct ISO8601 duration format.
234-244: LGTM! Comprehensive test coverage for new features.The test properly includes both
BillingCadenceandProRatingConfigfields, with appropriate values that align with the new pro-rating functionality.
389-390: LGTM! Tests plan update scenarios with billing cadence.The inclusion of
BillingCadencein the plan update test ensures this field can be properly modified, which is important for plan evolution scenarios.
439-441: LGTM! Thorough assertions for new subscription fields.The test properly validates that both
BillingCadenceandProRatingConfigfields are correctly persisted and retrieved, ensuring end-to-end functionality.
497-499: LGTM! Consistent testing across subscription types.The assertions ensure that plan-based subscriptions properly inherit billing cadence and pro-rating configuration from their plans, maintaining feature consistency.
640-643: LGTM! Complete field coverage for plan versioning.The test ensures that new plan versions include all necessary fields including
BillingCadence, which is important for maintaining consistency across plan versions.openmeter/ent/db/subscription/subscription.go (5)
10-11: LGTM! Necessary imports for new field types.The added imports support the new field type scanner and
ProRatingConfigtype, which are required for the schema enhancements.
45-48: LGTM! Consistent field constant definitions.The new field constants follow the established naming convention and are properly integrated with the existing schema structure.
114-115: LGTM! Proper column registration for new fields.The new fields are correctly added to the
Columnsslice, which is essential for ent's SQL query generation and field validation.
147-154: LGTM! Proper default value and scanner setup.The
DefaultProRatingConfigfunction andValueScannerstruct are correctly implemented to handle JSON serialization/deserialization of the pro-rating configuration field.
225-233: LGTM! Consistent ordering function implementation.The new ordering functions follow the established pattern and provide the ability to sort query results by the new billing cadence and pro-rating configuration fields.
openmeter/ent/schema/productcatalog.go (4)
5-5: LGTM! Necessary import for database dialect specification.The
dialectimport is required for specifying PostgreSQL JSONB schema type for the pro-rating configuration field.
36-38: LGTM! Well-defined billing cadence field.The field properly uses
isodate.StringGo type for ISO8601 duration handling and includes a clear descriptive comment.
39-51: LGTM! Comprehensive and well-structured field definition.The pro-rating configuration field is excellently implemented with:
- Proper Go type mapping to
productcatalog.ProRatingConfig- Custom value scanner for JSON serialization
- Sensible default values (enabled with prorate_prices mode)
- Efficient JSONB storage for PostgreSQL
- Clear and descriptive comment
This demonstrates best practices for complex field types in ent schemas.
200-200: LGTM! Consistent value scanner implementation.The
ProRatingConfigValueScannerfollows the established pattern for JSON field serialization, maintaining consistency with other complex field types in the schema.openmeter/ent/db/plan/plan.go (6)
10-11: LGTM: Imports added for new field functionality.The new imports for
fieldandproductcatalogpackages are necessary to support the ProRatingConfig field type and value scanning functionality.
41-44: LGTM: Field constants follow established naming convention.The new field constants
FieldBillingCadenceandFieldProRatingConfigconsistently follow the existing naming pattern and match the database column names.
94-95: LGTM: Fields properly added to Columns slice.The new fields are correctly included in the Columns slice, maintaining the proper order and ensuring they're recognized as valid columns for queries.
129-130: LGTM: Default function follows ENT patterns.The
DefaultProRatingConfigfunction declaration follows the established pattern for default value functions in ENT-generated code.
133-137: LGTM: ValueScanner properly configured for custom type.The ValueScanner struct with the ProRatingConfig field is correctly set up to handle the custom
productcatalog.ProRatingConfigtype, which is necessary for proper serialization/deserialization with the database.
197-205: LGTM: Ordering functions follow established patterns.The
ByBillingCadenceandByProRatingConfigordering functions correctly implement the same pattern as existing ordering functions, usingsql.OrderByFieldwith the appropriate field constants.tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql (5)
1-3: LGTM: Safe column addition pattern.Adding columns as nullable first is the correct approach to avoid lock issues and enable safe data population before making them NOT NULL.
34-38: LGTM: Sensible default for plans without rate cards.Setting 'P1M' (monthly) as the default billing cadence is a reasonable business decision. The comment correctly notes that defaults are set for ALL plans including soft-deleted ones to satisfy the NOT NULL constraint.
40-44: LGTM: Appropriate default pro-rating configuration.The default JSON configuration
{"mode": "prorate_prices", "enabled": true}enables pro-rating with a sensible mode, which aligns with typical billing system expectations.
56-86: Review subscription billing cadence logic.The subscription logic is more complex due to the phase ordering requirement but appears correct:
- Uses ROW_NUMBER() with ORDER BY active_from DESC to find the last phase
- Similar filtering for soft-deleted records
- Same shortest cadence selection logic
The approach is consistent with the plans logic and handles the subscription phase model appropriately.
88-102: LGTM: Consistent defaults and constraint application.The subscription defaults match the plan defaults, ensuring consistency across the system. The final ALTER TABLE statements correctly make both columns NOT NULL after data population.
openmeter/ent/db/migrate/schema.go (4)
2000-2001: New billing fields added to subscriptions table - consistent with plans.The billing cadence and pro-rating configuration fields are consistently defined across both plans and subscriptions tables, which maintains schema coherence. The use of PostgreSQL JSONB schema type for
pro_rating_configis appropriate for storing structured configuration data.
2013-2019: Foreign key column indices correctly updated after schema changes.The foreign key references have been properly updated to reflect the new column positions after adding
billing_cadenceandpro_rating_configfields:
customer_idforeign key moved from index 12 → 14plan_idforeign key moved from index 13 → 15This maintains referential integrity after the schema modification.
2043-2043: Index column reference correctly updated for customer relationship.The
subscription_namespace_customer_idindex has been properly updated to use column index 14 (previously 12) for thecustomer_idfield, maintaining the index functionality after the new columns were inserted.
1730-1731:✅ Verification successful
New billing fields added to plans table - verify migration handling.
The new
billing_cadenceandpro_rating_configcolumns are defined as non-nullable strings. Since these are new required fields being added to an existing table, ensure that the corresponding migration scripts properly handle existing records by providing default values or appropriate data population logic.Verify that the migration scripts handle existing plan records properly:
🏁 Script executed:
#!/bin/bash # Check if migration scripts exist and handle the new non-nullable columns fd -t f ".*plan.*billing.*cadence.*" tools/migrate/migrations/ fd -t f ".*subscription.*billing.*cadence.*" tools/migrate/migrations/Length of output: 509
🏁 Script executed:
#!/bin/bash # Inspect new column definitions in the up migration grep -nE 'ADD COLUMN' tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql grep -nE 'DEFAULT' tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql # Confirm drop statements in the down migration grep -nE 'DROP COLUMN' tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.down.sqlLength of output: 803
🏁 Script executed:
#!/bin/bash # Check for NOT NULL constraints in the up migration grep -nE 'NOT NULL' tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sqlLength of output: 900
Migration handles default values for new non-nullable billing fields
The up-migration script populates defaults for all existing
plansandsubscriptionsbefore applyingSET NOT NULLtobilling_cadenceandpro_rating_config. No further changes are needed.• tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql
– Update statements and comments around lines 35–48 and 89–102
– ALTER COLUMN … SET NOT NULL applied only after defaults are setopenmeter/subscription/subscriptionspec.go (2)
35-39: LGTM! Well-structured field additions.The new
BillingCadenceandProRatingConfigfields are properly typed, documented, and follow the existing JSON tag conventions.
65-70: LGTM! Correct field mapping.The new fields are properly mapped to the
CreateSubscriptionEntityInputstruct.openmeter/productcatalog/subscription/http/mapping.go (3)
196-200: LGTM! Proper API mapping for new fields.The billing cadence is correctly converted to string format and the pro-rating configuration is properly mapped with appropriate type conversion.
456-479: LGTM! Robust parsing with proper error handling.The billing cadence parsing includes appropriate error handling with a descriptive error message. The use of the
asProRatingConfighelper function ensures consistent handling of the pro-rating configuration.
497-510: LGTM! Well-designed helper with sensible defaults.The function provides good backward compatibility by returning sensible defaults when the configuration is not provided. The default of enabling pro-rating with price proration mode is a reasonable choice.
openmeter/ent/db/plan_update.go (1)
19-20: LGTM! Generated code properly handles new fields.The generated update builders correctly implement setters for
BillingCadenceandProRatingConfigfields with:
- Proper validation for
ProRatingConfig- Correct value conversion using the ValueScanner pattern
- Consistent implementation across both
PlanUpdateandPlanUpdateOnebuildersAlso applies to: 143-169, 367-371, 420-429, 704-730, 941-945, 1011-1020
pkg/isodate/date.go (1)
62-99: LGTM! Robust period-to-hours conversion.The implementation correctly handles variable month lengths and uses decimal arithmetic to avoid precision issues. The comprehensive error handling ensures arithmetic operations are safe.
openmeter/productcatalog/plan.go (1)
106-118: LGTM! Clear and correct validation logic.The function correctly validates that the billing cadence is at least one month using the ISO date period comparison.
api/openapi.cloud.yaml (13)
13153-13158: RequiredbillingCadencefield added correctly
TheCustomPlanInputschema now requiresbillingCadence, matching the PR objectives and ensuring clients must specify billing cadence when creating a plan.
13187-13203: AddedbillingCadenceandproRatingConfigproperties for Plan schema
The plan schema now includes abillingCadencestring field with ISO8601 format and aproRatingConfigobject referencing the new component schema, complete with sensible defaults. This aligns with the API contract.
19125-19128: RequiredbillingCadencein Plan representation
ThePlanschema now mandates thebillingCadenceproperty in its response model, ensuring clients can rely on its presence.
19202-19219: Extended PlanReplaceUpdate withbillingCadenceandproRatingConfig
The update schema correctly defines and documents the new fields, including defaults for pro-rating configuration, allowing safe plan modifications.
19417-19420: RequiredbillingCadencein PlanCreate schema
Plan creation now enforces thebillingCadencefield, consistent with feature requirements and other plan endpoints.
19457-19473: DefinedbillingCadenceandproRatingConfigin PlanCreate
ThePlanCreateschema now includes the cadence and pro-rating settings with clear documentation and defaults, ensuring new plans carry these settings out of the box.
19607-19608: EnforcedbillingCadencerequirement in replacement plan schema
This plan-replacement schema now correctly listsbillingCadenceas required, maintaining consistency across all plan-related definitions.
19632-19649: AddedbillingCadenceandproRatingConfigto alignment-based plan schema
The schema that composes alignment rules for plans now includes both new fields, ensuring alignment configurations respect billing cadence and pro-rating settings.
19840-19858: DefinedProRatingConfigandProRatingModeschemas
The new component schemas correctly specify the pro-rating configuration object and its enum mode, matching the API contract and providing a single default behavior.
20849-20852: IncludebillingCadencein Subscription summary
Good addition ofbillingCadenceto the subscription summary schema. Should we also consider markingproRatingConfigas required, since responses always include it?
20935-20944: AddedbillingCadenceandproRatingConfigto Subscription schema
TheSubscriptionschema now includes the new fields withreadOnly: trueand default values, aligning the subscription response with plan definitions.
21323-21327: RequiredbillingCadencein detailed Subscription schema
The expanded subscription view (SubscriptionExpanded) now enforcesbillingCadence, ensuring all detailed subscription endpoints expose the cadence field.
21407-21425: Extended SubscriptionExpanded with new fields
The expanded subscription schema correctly includesbillingCadenceandproRatingConfigwithreadOnly: trueand clear documentation, fulfilling the end-to-end feature requirements.openmeter/ent/db/subscription_update.go (1)
1-1203: LGTM! Auto-generated code follows consistent patterns.The auto-generated update builders correctly implement:
- Setter methods for the new
billing_cadenceandpro_rating_configfields- Validation for
pro_rating_configusing itsValidate()method- Proper value conversion using the
ValueScannerfor complex types- Consistent implementation across both
SubscriptionUpdateandSubscriptionUpdateOnebuildersopenmeter/productcatalog/plan/httpdriver/mapping.go (2)
156-169: Verify that the default pro-rating configuration aligns with business requirements.The function returns default values when the API input is nil:
Enabled: trueMode: productcatalog.ProRatingModeProratePricesPlease ensure these defaults match the intended behavior for backward compatibility and new subscriptions.
33-34: Excellent implementation of the mapping layer.The HTTP mapping correctly handles:
- Conversion between domain and API representations for both fields
- Error handling for ISO8601 duration parsing
- Nil safety with sensible defaults for pro-rating configuration
- Consistent patterns across create and update operations
Also applies to: 105-105, 126-129, 205-205, 214-221
openmeter/ent/db/plan/where.go (1)
119-123: LGTM! Auto-generated predicates follow established patterns.The billing cadence predicates correctly:
- Convert
isodate.Stringtostringbefore applying SQL predicates- Provide comprehensive query operations (EQ, NEQ, In, GT, Contains, etc.)
- Match the implementation pattern used in related entities (Subscription, PlanRateCard)
Also applies to: 660-742
tools/migrate/productcatalog_test.go (3)
5-12: LGTM! Imports are well-organized and necessary.The added imports are properly used throughout the test functions for JSON unmarshaling, string formatting, and accessing productcatalog types.
180-732: Excellent test coverage for plan billing cadence migration!The test thoroughly covers:
- Multiple scenarios with different rate card configurations
- PostgreSQL interval casting verification (critical for ORDER BY logic)
- Both up and down migration paths
- Proper JSON unmarshaling and validation of pro-rating config
The detailed inline comments explaining each test scenario are particularly helpful for understanding the expected behavior.
734-1274: Comprehensive test coverage for subscription billing cadence migration!The test effectively mirrors the plan migration test structure while properly adapting it for subscriptions:
- Creates required customers to satisfy foreign key constraints
- Uses subscription items instead of rate cards
- Maintains the same thorough testing approach
The parallel structure between this test and the plan test makes the codebase more maintainable.
openmeter/ent/db/subscription/where.go (1)
1-1019: Auto-generated code looks correct.This file is auto-generated by ent and follows the expected pattern for predicate functions. The new
billing_cadencepredicates (lines 131-888) are properly implemented with all standard operations and correctly convert fromisodate.Stringto string before applying SQL predicates.api/client/javascript/src/zod/index.ts (3)
11064-11080: Well-structured proRatingConfig schema with appropriate defaults.The proRatingConfig object definition is well-structured with clear descriptions and sensible defaults. The enum restriction to 'prorate_prices' suggests this is the initial implementation with potential for future expansion.
14855-14873: Consistent schema structure for subscription proRatingConfig.The proRatingConfig implementation in subscription schemas maintains consistency with the plan schemas, which is good for API coherence.
17342-17360: Comprehensive schema coverage across all plan and subscription operations.The addition of proRatingConfig to all relevant schemas (create plan, update plan, create subscription with custom plan, and change subscription with custom plan) ensures comprehensive coverage of the new billing features.
openmeter/ent/db/mutation.go (13)
38672-38673: Field declarations follow correct ent patterns.The new fields
billing_cadenceandpro_rating_configare properly declared as pointer types, which is consistent with other mutation fields in the struct.
39250-39284: BillingCadence field methods are correctly implemented.The setter, getter, old value retrieval, and reset methods for
billing_cadencefollow the exact same patterns as other fields in the mutation, ensuring consistency with ent conventions.
39286-39320: ProRatingConfig field methods are correctly implemented.The setter, getter, old value retrieval, and reset methods for
pro_rating_configare properly implemented and consistent with ent patterns.
39616-39616: Field count updated correctly for PlanMutation.The field count is updated to 15, which accounts for the addition of the two new fields (
billing_cadenceandpro_rating_config).
39650-39655: New fields properly added to Fields() method.Both
billing_cadenceandpro_rating_configfields are correctly added to the Fields() method with proper field constant references.
39692-39695: Field cases added to Field() method correctly.The new field cases return the appropriate getter methods and follow the same pattern as existing fields.
39731-39734: Old field cases implemented properly.The OldField() method correctly handles the new fields by calling their respective Old* methods.
39825-39838: Type assertions and field setting handled correctly.The SetField() method properly type-checks the values and calls the appropriate setter methods. The type assertions match the expected types (
isodate.Stringandproductcatalog.ProRatingConfig).
39983-39988: Reset field cases implemented correctly.The ResetField() method properly handles the new fields by calling their respective Reset* methods.
38814-38815: SubscriptionMutation field declarations are correct.The new fields are properly declared in the SubscriptionMutation struct, maintaining consistency with the PlanMutation implementation.
44472-44506: SubscriptionMutation field methods implemented correctly.All setter, getter, old value retrieval, and reset methods for both fields in SubscriptionMutation follow the same correct patterns as in PlanMutation.
Also applies to: 44508-44542
44794-44794: SubscriptionMutation field count updated correctly.The field count is properly updated to 15 for SubscriptionMutation as well.
44834-44839: All SubscriptionMutation field handling methods updated correctly.The Fields(), Field(), OldField(), SetField(), and ResetField() methods are all properly updated to handle the new fields with consistent patterns and correct type handling.
Also applies to: 44874-44877, 44913-44916, 45017-45030, 45152-45157
api/client/javascript/src/client/schemas.ts (5)
3507-3524: Excellent implementation of billing configuration fields.The
billingCadenceandproRatingConfigfields are properly defined with comprehensive JSDoc documentation, appropriate types, and sensible defaults. The ISO8601 duration format for billing cadence is well-documented with clear examples.
8289-8306: Consistent implementation across PlanReplaceUpdate.The field definitions match other plan schemas perfectly, maintaining consistency in documentation, types, and default values.
9297-9314: Appropriate readonly fields for Subscription schema.The
readonlymodifiers are correctly applied since subscription billing configuration is typically derived from the plan and shouldn't be directly modifiable on the subscription object.
9666-9683: Consistent readonly implementation in SubscriptionExpanded.Matches the Subscription schema pattern with proper readonly modifiers and identical field definitions.
10915-10916: Export statements properly added for new types.The new
ProRatingConfigandProRatingModetypes are correctly exported, making them available for import by consuming code.
tools/migrate/migrations/20250527114615_plan-subscription-billing-cadence-pro-rating.up.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
pkg/isodate/date_test.go (1)
286-298: Fix duplicate test names.The test cases on lines 286 and 293 have identical names "smaller period larger than larger period 2", which could cause confusion when debugging test failures.
Apply this diff to make the test names unique:
{ - name: "smaller period larger than larger period 2", + name: "smaller period (1 week) larger than larger period (1 month)", larger: "P1W", smaller: "P1M", expected: false, hasError: false, }, { - name: "smaller period larger than larger period 2", + name: "smaller period (4 weeks) larger than larger period (1 month)", larger: "P4W", smaller: "P1M", expected: false, hasError: false, },openmeter/productcatalog/alignment.go (2)
33-34: Fix typo in comment.There's a typo in the comment: "DibisibleBy" should be "DivisibleBy".
-// We'll leverage the fact that Period.DibisibleBy() works correctly regardless which period is larger, +// We'll leverage the fact that Period.DivisibleBy() works correctly regardless which period is larger,
46-46: Enhance error message for better clarity.The error message could be more descriptive to indicate it's specifically an alignment validation error.
-return fmt.Errorf("billing cadences do not align: %s and %s", planBillingCadence, rateCardBillingCadence) +return fmt.Errorf("%w: plan billing cadence %s is not compatible with rate card billing cadence %s", ErrPlanBillingCadenceNotCompatible, planBillingCadence, rateCardBillingCadence)This assumes
ErrPlanBillingCadenceNotCompatibleis defined in the errors file as mentioned in the AI summary.pkg/isodate/date.go (1)
63-115: Consider documenting the year calculation approach more clearly.The comment on lines 67-70 acknowledges that the year calculation is unconventional (using
daysInMonth*12*24hours). While you claim this gives correct results, it would be helpful to:
- Explain why this approach works despite appearing incorrect
- Add unit tests demonstrating correctness for edge cases
- Consider adding a more detailed comment or documentation
openmeter/billing/worker/subscription/phaseiterator.go (1)
279-283: Minor: Consider combining increment operations.The increment logic is split across lines, which could be slightly cleaner:
*items = append(*items, newItem) -// Let's increment -periodIdx = periodIdx + 1 +// Advance to next period +periodIdx++ at = newItem.ServicePeriod.End
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
openmeter/billing/invoiceline.go(3 hunks)openmeter/billing/worker/subscription/phaseiterator.go(13 hunks)openmeter/billing/worker/subscription/phaseiterator_test.go(18 hunks)openmeter/billing/worker/subscription/sync.go(9 hunks)openmeter/billing/worker/subscription/sync_test.go(16 hunks)openmeter/productcatalog/alignment.go(2 hunks)openmeter/productcatalog/errors.go(2 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/price.go(2 hunks)openmeter/productcatalog/ratecard.go(2 hunks)openmeter/subscription/addon/extend_test.go(1 hunks)openmeter/subscription/subscription.go(3 hunks)openmeter/subscription/subscriptionspec.go(8 hunks)openmeter/subscription/subscriptionview.go(2 hunks)openmeter/subscription/testutils/compare.go(1 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)pkg/timeutil/closedperiod.go(1 hunks)pkg/timeutil/closedperiod_test.go(1 hunks)pkg/timeutil/openperiod.go(2 hunks)pkg/timeutil/recurrence.go(1 hunks)test/billing/subscription_test.go(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- openmeter/subscription/addon/extend_test.go
- pkg/timeutil/recurrence.go
- openmeter/billing/invoiceline.go
- openmeter/subscription/testutils/compare.go
🚧 Files skipped from review as they are similar to previous changes (7)
- openmeter/subscription/subscriptionview.go
- openmeter/subscription/subscription.go
- test/billing/subscription_test.go
- openmeter/productcatalog/plan.go
- openmeter/productcatalog/errors.go
- openmeter/billing/worker/subscription/sync_test.go
- openmeter/subscription/subscriptionspec.go
🧰 Additional context used
🧬 Code Graph Analysis (7)
openmeter/productcatalog/ratecard.go (1)
openmeter/productcatalog/price.go (1)
Price(86-93)
pkg/timeutil/closedperiod.go (1)
pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
pkg/timeutil/openperiod.go (1)
pkg/timeutil/closedperiod.go (1)
ClosedPeriod(7-10)
openmeter/productcatalog/price.go (3)
api/api.gen.go (3)
PaymentTermType(5452-5452)Price(5826-5828)FlatPriceType(3189-3189)openmeter/billing/invoiceline.go (1)
Price(835-835)api/client/go/client.gen.go (1)
FlatPriceType(2894-2894)
openmeter/productcatalog/alignment.go (1)
pkg/isodate/date.go (1)
Period(49-51)
pkg/timeutil/closedperiod_test.go (1)
pkg/timeutil/closedperiod.go (1)
ClosedPeriod(7-10)
openmeter/billing/worker/subscription/phaseiterator.go (13)
openmeter/subscription/subscriptionview.go (1)
SubscriptionItemView(99-105)openmeter/billing/invoiceline.go (2)
Period(84-87)Price(835-835)api/client/go/client.gen.go (5)
Period(5004-5010)RateCard(5409-5411)FlatPriceType(2894-2894)Subscription(5908-5959)SubscriptionItem(6186-6246)openmeter/productcatalog/ratecard.go (1)
RateCard(29-42)openmeter/productcatalog/price.go (3)
Price(86-93)FlatPriceType(41-41)InAdvancePaymentTerm(20-20)pkg/framework/tracex/tracex.go (1)
Start(62-66)openmeter/ent/db/subscription.go (2)
Subscription(22-60)Subscription(129-148)openmeter/ent/schema/subscription.go (10)
Subscription(18-20)Subscription(22-31)Subscription(33-57)Subscription(59-64)Subscription(66-77)SubscriptionItem(118-120)SubscriptionItem(122-129)SubscriptionItem(131-194)SubscriptionItem(196-201)SubscriptionItem(203-209)openmeter/ent/db/subscriptionitem.go (2)
SubscriptionItem(21-73)SubscriptionItem(120-147)openmeter/subscription/item.go (1)
SubscriptionItem(12-44)pkg/models/cadence.go (1)
CadencedModel(21-25)pkg/timeutil/compare.go (1)
Compare(5-7)openmeter/billing/defaults.go (1)
DefaultMeterResolution(12-12)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: E2E
- GitHub Check: Quickstart
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Analyze (go)
🔇 Additional comments (18)
openmeter/productcatalog/ratecard.go (2)
41-41: LGTM! Clean interface extension.The addition of the
IsBillable()method to theRateCardinterface provides a clear way to determine if a rate card contributes to billing logic.
227-229: LGTM! Correct implementation.The implementation correctly determines billability by checking if the
Pricefield is non-nil. This aligns with the business logic where rate cards without prices should not contribute to billing calculations.pkg/timeutil/openperiod.go (2)
3-6: LGTM! Necessary import addition.The
fmtpackage import is correctly added to support the error formatting in the newClosed()method.
270-279: LGTM! Proper validation and conversion logic.The
Closed()method correctly validates that bothFromandToboundaries are non-nil before creating aClosedPeriod. This prevents invalid conversions and provides a clear error message when conversion isn't possible.pkg/timeutil/closedperiod.go (2)
57-84: LGTM! Correct intersection algorithm.The
Intersection()method correctly implements the period intersection logic:
- Takes the later of the two start times
- Takes the earlier of the two end times
- Validates that the result forms a valid period
- Returns
nilwhen periods don't overlapThe implementation handles all edge cases properly.
86-91: LGTM! Proper conversion implementation.The
Open()method correctly converts aClosedPeriodto anOpenPeriodby taking addresses of theFromandTofields. This creates the expected pointer-based structure forOpenPeriod.openmeter/productcatalog/price.go (2)
81-81: LGTM! Logical interface extension.The addition of
GetPaymentTerm()to thepricerinterface provides a consistent way to retrieve payment terms across different price types.
410-418: LGTM! Sensible implementation with clear logic.The implementation correctly returns the payment term for flat prices (which support configurable payment terms) and defaults to
InArrearsPaymentTermfor other price types. The comment clearly explains this design decision.pkg/timeutil/closedperiod_test.go (1)
126-188: LGTM! Comprehensive test coverage for the Intersection method.The test cases thoroughly cover all scenarios for period intersection including edge cases. The test structure is consistent with the existing codebase patterns.
pkg/isodate/date.go (1)
59-61: LGTM!The
Simplifymethod is a clean wrapper around the underlying library's functionality.openmeter/billing/worker/subscription/sync.go (2)
236-242: Good refactoring to support multiple period types.The transition from
subscriptionItemWithPeriodtosubscriptionItemWithPeriodswith distinctServicePeriod,FullServicePeriod, andBillingPeriodprovides better flexibility for handling different billing scenarios. The logic for in-arrears items appropriately waits until the full service period is known.Also applies to: 466-483
554-576: Well-structured proration logic.The
shouldProratemethod properly centralizes proration decisions with clear checks:
- Subscription-level configuration
- Price type validation
- Subscription end date handling
- Mode-based behavior
This provides good flexibility and maintainability.
openmeter/billing/worker/subscription/phaseiterator_test.go (1)
37-42: Comprehensive test coverage for the new multi-period structure.The test updates properly validate the new
ServicePeriod,FullServicePeriod, andBillingPeriodstructure across various scenarios including:
- Aligned and non-aligned subscriptions
- Different billing cadences
- Phase and subscription end dates
- Edge cases with period boundaries
The test cases provide good coverage for the billing cadence and proration features.
Also applies to: 97-925
openmeter/billing/worker/subscription/phaseiterator.go (5)
45-60: Good refactoring to separate period concerns!The introduction of distinct period types (
ServicePeriod,FullServicePeriod, andBillingPeriod) improves code clarity by explicitly modeling different billing concepts. The documentation clearly explains the purpose of each field.
64-74: Correct adaptation to new period structure.The method properly uses the new
FullServicePeriodandServicePeriodfields to calculate the percentage for proration. The zero-length check prevents division by zero errors.
76-92: Well-designed invoice timing logic.The
GetInvoiceAtmethod effectively consolidates invoice date calculation with clear handling of in-advance vs after-the-fact billing. The use oflo.Latestensures proper timing constraints are respected.
324-388: Consistent and correct period calculation across generation methods.All generation methods follow a consistent pattern:
- Calculate
FullServicePeriodbased on item configuration- Derive
ServicePeriodas the intersection with item cadence- Determine
BillingPeriodbased on alignment requirementsThe edge case handling for zero-duration periods (instant billing) is properly implemented.
Also applies to: 391-501, 525-593, 595-651
503-523: Correct truncation logic for meter resolution.The truncation appropriately targets only the
ServicePeriodand correctly skips flat-priced items that don't require meter resolution alignment. Filtering out empty periods after truncation prevents invalid billing items.
GAlexIHU
force-pushed
the
feat/plan-billing-cadence
branch
from
dd5b5ab to
8a0c51c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pkg/isodate/date.go (1)
163-163: Fix error return value when periods are not divisible.This issue was already identified in a previous review. The method returns
false, errwhereerrwould benil. This should returnfalse, nil.
🧹 Nitpick comments (2)
openmeter/billing/worker/subscription/sync_test.go (2)
804-839: Consider extracting common plan configuration to reduce duplication.This large block creates a plan input with the same billing configuration fields that appear in multiple tests. Consider creating a helper method to reduce code duplication.
+func (s *SubscriptionHandlerTestSuite) createBasePlanInput() plan.CreatePlanInput { + return plan.CreatePlanInput{ + NamespacedModel: models.NamespacedModel{ + Namespace: s.Namespace, + }, + Plan: productcatalog.Plan{ + PlanMeta: productcatalog.PlanMeta{ + Name: "Test Plan", + Key: "test-plan", + Version: 1, + Currency: currency.USD, + BillingCadence: isodate.MustParse(s.T(), "P1M"), + ProRatingConfig: productcatalog.ProRatingConfig{ + Enabled: false, + Mode: productcatalog.ProRatingModeProratePrices, + }, + }, + Phases: nil, // to be set by caller + }, + } +}
3611-3831: Complex but comprehensive test - consider breaking down for maintainability.This new test function
TestAlignedSubscriptionProratingBehavioris quite extensive (220+ lines) and tests multiple complex scenarios. While the test logic appears correct, consider breaking it down into smaller, focused test functions for better maintainability.The test correctly verifies:
- Prorated amounts for the first phase (2.26 = 5 * 14/31 days)
- Different behavior for in-advance vs in-arrears payment terms
- Proper alignment of second phase billing periods
- Correct invoice timing based on payment terms
Consider splitting into:
TestFirstPhaseProratingTestSecondPhaseAlignmentTestMultiPhaseInvoiceTiming
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (79)
api/client/javascript/src/client/schemas.ts(8 hunks)api/client/javascript/src/zod/index.ts(12 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/package.json(1 hunks)api/spec/src/productcatalog/plan.tsp(2 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)e2e/productcatalog_test.go(7 hunks)openmeter/billing/invoiceline.go(3 hunks)openmeter/billing/worker/subscription/phaseiterator.go(13 hunks)openmeter/billing/worker/subscription/phaseiterator_test.go(18 hunks)openmeter/billing/worker/subscription/sync.go(9 hunks)openmeter/billing/worker/subscription/sync_test.go(16 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(16 hunks)openmeter/ent/db/plan.go(5 hunks)openmeter/ent/db/plan/plan.go(5 hunks)openmeter/ent/db/plan/where.go(3 hunks)openmeter/ent/db/plan_create.go(11 hunks)openmeter/ent/db/plan_update.go(7 hunks)openmeter/ent/db/predicate/predicate.go(2 hunks)openmeter/ent/db/runtime.go(2 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(5 hunks)openmeter/ent/db/subscription/where.go(3 hunks)openmeter/ent/db/subscription_create.go(12 hunks)openmeter/ent/db/subscription_update.go(7 hunks)openmeter/ent/schema/productcatalog.go(3 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/adapter/feature_test.go(1 hunks)openmeter/productcatalog/addon/adapter/mapping.go(2 hunks)openmeter/productcatalog/alignment.go(2 hunks)openmeter/productcatalog/errors.go(2 hunks)openmeter/productcatalog/plan.go(5 hunks)openmeter/productcatalog/plan/adapter/mapping.go(2 hunks)openmeter/productcatalog/plan/adapter/plan.go(2 hunks)openmeter/productcatalog/plan/httpdriver/mapping.go(6 hunks)openmeter/productcatalog/plan/serializer_test.go(1 hunks)openmeter/productcatalog/plan/service.go(3 hunks)openmeter/productcatalog/plan/service/plan.go(1 hunks)openmeter/productcatalog/plan/service/service_test.go(1 hunks)openmeter/productcatalog/plan_test.go(13 hunks)openmeter/productcatalog/planaddon_test.go(3 hunks)openmeter/productcatalog/price.go(2 hunks)openmeter/productcatalog/pro_rating.go(1 hunks)openmeter/productcatalog/ratecard.go(2 hunks)openmeter/productcatalog/subscription/http/mapping.go(4 hunks)openmeter/productcatalog/subscription/plan.go(1 hunks)openmeter/productcatalog/subscription/service/plan.go(1 hunks)openmeter/productcatalog/testutils/plan.go(2 hunks)openmeter/subscription/addon/diff/restore_test.go(2 hunks)openmeter/subscription/addon/extend_test.go(1 hunks)openmeter/subscription/aligment_test.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(1 hunks)openmeter/subscription/repository.go(1 hunks)openmeter/subscription/subscription.go(3 hunks)openmeter/subscription/subscriptionspec.go(8 hunks)openmeter/subscription/subscriptionview.go(2 hunks)openmeter/subscription/testutils/compare.go(1 hunks)openmeter/subscription/testutils/plan.go(1 hunks)openmeter/subscription/workflow/service/addon_test.go(2 hunks)openmeter/subscription/workflow/service/subscription.go(1 hunks)openmeter/subscription/workflow/service/subscription_test.go(2 hunks)pkg/isodate/date.go(3 hunks)pkg/isodate/date_test.go(1 hunks)pkg/timeutil/closedperiod.go(1 hunks)pkg/timeutil/closedperiod_test.go(1 hunks)pkg/timeutil/openperiod.go(2 hunks)pkg/timeutil/recurrence.go(1 hunks)test/billing/subscription_test.go(2 hunks)test/customer/customer.go(3 hunks)test/subscription/scenario_editaligned_test.go(3 hunks)test/subscription/scenario_editcancel_test.go(2 hunks)tools/migrate/entitlement_test.go(5 hunks)tools/migrate/migrations/20250606130010_plan-subscription-billing-cadence.down.sql(1 hunks)tools/migrate/migrations/20250606130010_plan-subscription-billing-cadence.up.sql(1 hunks)tools/migrate/productcatalog_test.go(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- tools/migrate/migrations/20250606130010_plan-subscription-billing-cadence.down.sql
- openmeter/subscription/workflow/service/addon_test.go
- tools/migrate/entitlement_test.go
🚧 Files skipped from review as they are similar to previous changes (70)
- api/spec/package.json
- openmeter/productcatalog/plan/serializer_test.go
- openmeter/productcatalog/adapter/feature_test.go
- openmeter/subscription/aligment_test.go
- openmeter/productcatalog/testutils/plan.go
- openmeter/subscription/workflow/service/subscription_test.go
- openmeter/productcatalog/subscription/service/plan.go
- openmeter/productcatalog/plan/service/plan.go
- openmeter/subscription/testutils/compare.go
- openmeter/subscription/testutils/plan.go
- api/spec/src/productcatalog/prorating.tsp
- openmeter/subscription/addon/diff/restore_test.go
- pkg/timeutil/openperiod.go
- pkg/timeutil/recurrence.go
- openmeter/subscription/repository.go
- test/customer/customer.go
- test/subscription/scenario_editcancel_test.go
- openmeter/subscription/repo/mapping.go
- test/subscription/scenario_editaligned_test.go
- openmeter/subscription/repo/subscriptionrepo.go
- openmeter/subscription/subscriptionview.go
- openmeter/ent/db/predicate/predicate.go
- test/billing/subscription_test.go
- openmeter/productcatalog/ratecard.go
- openmeter/productcatalog/addon/adapter/mapping.go
- openmeter/productcatalog/price.go
- pkg/isodate/date_test.go
- openmeter/productcatalog/plan/service/service_test.go
- openmeter/subscription/addon/extend_test.go
- pkg/timeutil/closedperiod.go
- pkg/timeutil/closedperiod_test.go
- openmeter/ent/schema/subscription.go
- openmeter/productcatalog/pro_rating.go
- openmeter/billing/invoiceline.go
- openmeter/productcatalog/plan/adapter/plan.go
- openmeter/productcatalog/alignment.go
- openmeter/subscription/workflow/service/subscription.go
- openmeter/ent/db/subscription.go
- openmeter/productcatalog/subscription/plan.go
- openmeter/productcatalog/errors.go
- api/spec/src/productcatalog/subscription.tsp
- api/spec/src/productcatalog/plan.tsp
- e2e/productcatalog_test.go
- openmeter/ent/db/plan.go
- openmeter/productcatalog/plan/adapter/mapping.go
- openmeter/productcatalog/plan.go
- openmeter/productcatalog/plan_test.go
- openmeter/ent/db/subscription/subscription.go
- openmeter/productcatalog/subscription/http/mapping.go
- openmeter/ent/db/runtime.go
- openmeter/productcatalog/planaddon_test.go
- openmeter/ent/schema/productcatalog.go
- openmeter/ent/db/migrate/schema.go
- openmeter/ent/db/plan/plan.go
- openmeter/subscription/subscription.go
- openmeter/productcatalog/plan/service.go
- openmeter/ent/db/subscription_update.go
- api/openapi.cloud.yaml
- openmeter/ent/db/plan_update.go
- openmeter/productcatalog/plan/httpdriver/mapping.go
- api/client/javascript/src/zod/index.ts
- api/openapi.yaml
- openmeter/ent/db/plan/where.go
- api/client/javascript/src/client/schemas.ts
- openmeter/ent/db/subscription/where.go
- openmeter/ent/db/plan_create.go
- openmeter/ent/db/subscription_create.go
- openmeter/ent/db/mutation.go
- openmeter/subscription/subscriptionspec.go
- openmeter/billing/worker/subscription/sync.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
tools/migrate/productcatalog_test.go (2)
api/client/javascript/src/client/schemas.ts (1)
ProRatingConfig(10935-10935)openmeter/productcatalog/pro_rating.go (2)
ProRatingConfig(23-29)ProRatingModeProratePrices(11-11)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Test
- GitHub Check: Quickstart
- GitHub Check: Migration Checks
- GitHub Check: Lint
- GitHub Check: E2E
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: Build
- GitHub Check: Commit hooks
- GitHub Check: Analyze (go)
🔇 Additional comments (35)
pkg/isodate/date.go (2)
59-61: LGTM!The
Simplifymethod correctly delegates to the underlying Period implementation.
63-115: LGTM! Well-implemented period to hours conversion.The method correctly handles decimal arithmetic with proper error checking throughout. The detailed comment explains the non-intuitive year calculation approach.
tools/migrate/migrations/20250606130010_plan-subscription-billing-cadence.up.sql (2)
1-49: Well-structured migration approach for plans table.The migration correctly:
- Adds nullable columns first to avoid constraint violations
- Uses PostgreSQL interval casting to find the shortest billing cadence
- Handles soft-deleted records appropriately by setting defaults for all rows
- Sets sensible defaults (P1M and enabled prorate_prices)
50-103: Consistent migration approach for subscriptions table.The migration correctly identifies the last subscription phase using
ROW_NUMBER() OVER (PARTITION BY subscription_id ORDER BY active_from DESC)and applies the same shortest billing cadence selection logic as the plans migration.tools/migrate/productcatalog_test.go (2)
180-733: Excellent comprehensive test coverage for plan migration.The test thoroughly validates:
- PostgreSQL's interval casting and comparison capabilities
- Correct selection of shortest billing cadence from rate cards
- Default value application for plans without rate cards
- Proper JSON structure for pro_rating_config
- Clean rollback in down migration
735-1275: Thorough test coverage for subscription migration.The test properly:
- Sets up required customer data for foreign key constraints
- Validates phase ordering by active_from timestamp
- Confirms shortest billing cadence selection from subscription items
- Verifies the same pro_rating_config defaults as plans
openmeter/billing/worker/subscription/sync_test.go (7)
157-165: LGTM: Consistent addition of billing configuration fields.The addition of
BillingCadenceandProRatingConfigto thePlanMetastructure is consistent with the broader system changes. The values used (P1M cadence and prorate prices mode) are appropriate for the test scenario.
574-582: LGTM: Consistent billing configuration in prorating test.The same billing configuration fields are properly added here, maintaining consistency across test cases.
666-666: LGTM: Clear test documentation and correct assertions.The test comment clearly explains the expected behavior, and the assertions correctly verify that flat fee amounts remain unchanged (5.0 and 1.0) when prorating is disabled for in-arrears billing.
Also applies to: 709-710
1680-1680: LGTM: Invoice timing aligned with billing cadence.The invoice timestamps (2024-01-29T00:00:00Z) are correctly aligned with the 4-week billing cadence ("P4W") specified in the plan, showing proper integration of the billing cadence feature.
Also applies to: 1699-1699
3868-3881: Excellent defensive programming with nil checks.The addition of nil checks prevents potential panics when validating line quantities and prices. This is a solid improvement to the test infrastructure.
The logic properly handles different line types:
- Checks
FlatFeefor fee-type lines- Checks
UsageBased.Quantityfor usage-based lines- Provides clear error messages when expected data is missing
3885-3887: LGTM: Robust validation with clear error messages.The enhanced validation logic properly:
- Validates line types match expectations
- Provides descriptive error messages with child IDs
- Ensures period and invoice timing assertions are accurate
This improves the reliability of the test suite.
Also applies to: 3889-3892, 3894-3897
4079-4087: LGTM: Consistent helper method updates.The helper method properly includes the new billing configuration fields, maintaining consistency with the other plan creation patterns in the file.
openmeter/billing/worker/subscription/phaseiterator_test.go (8)
15-15: Import addition looks correct.The
billingpackage import is appropriately added to support the newbilling.Periodtype used in the refactored test structure.
37-42: Well-structured refactoring of test expectations.The change from simple start/end times to three distinct period types (
ServicePeriod,FullServicePeriod,BillingPeriod) properly reflects the new multi-period billing model. This provides better test coverage for the different period calculations.
62-69: Good addition of aligned billing test parameter.The replacement of the boolean
alignedSubflag with an explicitalignedBillingCadencefield of typeisodate.Periodprovides more precise control over test scenarios and better reflects the actual billing cadence configuration.
98-125: Comprehensive test expectation updates.The conversion of all expected test outputs to use the new three-period structure is thorough and consistent. Each period type is properly populated with the correct start/end times that align with the test scenario logic.
1017-1031: Robust test setup logic for phase and subscription end handling.The logic to adjust subscription item active-to dates based on phase and subscription end times is well-implemented. It correctly handles the precedence where earlier end times take priority, ensuring test scenarios reflect realistic subscription lifecycle constraints.
1065-1070: Proper test setup for aligned billing scenarios.The conditional setup of billing cadence fields when alignment is specified correctly reflects the subscription view changes. The assignment of both spec and subscription level billing cadence ensures consistency in the test data.
1098-1101: Appropriate test output processing update.The conversion of generated output to the new
expectedIterationsstructure using the three period fields is correct and maintains test assertion compatibility.
1103-1108: Enhanced test logging for better debugging.The updated logging statements now include all three period types in both actual and expected outputs, providing comprehensive information for test debugging and validation.
openmeter/billing/worker/subscription/phaseiterator.go (14)
45-60: Well-designed multi-period structure.The new
subscriptionItemWithPeriodsstruct properly encapsulates the three distinct period types needed for advanced billing scenarios. The clear documentation of each period's purpose (ServicePeriod,FullServicePeriod,BillingPeriod) helps maintainability.
64-74: Robust period percentage calculation.The
PeriodPercentage()method correctly handles the edge case where the full service period is empty by returning 100% to prevent unintended proration. The calculation using the duration ratio is mathematically sound for pro-rating scenarios.
76-92: Correct invoice timing logic.The
GetInvoiceAt()method properly implements the business logic where flat-fee in-advance items are invoiced at the start of their periods, while all other items are invoiced after their service/billing periods complete. The use oflo.Latest()ensures correct timing constraints.
186-198: Appropriate method signature update.The change to return
[]subscriptionItemWithPeriodscorrectly reflects the new multi-period model. The conditional logic to choose between aligned and non-aligned generation is clean and follows the subscription configuration.
299-301: Improved iteration control logic.The updated loop exit condition now uses the new
GetInvoiceAt()method instead of a separategetInvoiceAtfunction, providing better encapsulation and consistency with the period model.
343-352: Enhanced period calculation for aligned billing.The call to
GetFullServicePeriodAt()with the billing period start as an alignment reference is a good approach for ensuring consistent period boundaries in aligned billing scenarios.
354-358: Robust service period intersection logic.The calculation of service period as the intersection of full service period and item cadence properly handles the boundary constraints. The error handling ensures that invalid intersections are caught early.
373-385: Complete period assignment in generated items.The assignment of all three period types to the generated item ensures that downstream billing logic has access to the full period context needed for accurate billing and pro-rating calculations.
429-437: Consistent period calculation in non-aligned generation.The use of
GetFullServicePeriodAt()in non-aligned generation maintains consistency with the aligned version while properly handling the case where no alignment reference is needed (passingnil).
439-448: Appropriate billing period logic for non-aligned items.The logic where non-aligned billing periods follow the full service period (with phase boundary constraints) is correct. This ensures that non-aligned subscriptions maintain their natural cadence while respecting phase limits.
514-518: Correct truncation logic update.The change to truncate the
ServicePeriodinstead of a generic period properly reflects that meter resolution constraints apply specifically to the actual service period being billed.
553-567: Robust handling of edge cases in service period calculation.The logic to handle instant service periods (duration of 0) and the comprehensive error handling for empty service periods demonstrates good defensive programming practices. The detailed error messages will help with debugging period calculation issues.
569-592: Complete aligned one-time item generation.The generation of all three period types for aligned one-time items ensures consistent behavior with recurring items. The unique ID construction properly identifies the item version and context.
624-625: Logical billing period assignment for one-time items.Setting the billing period equal to the full service period for one-time items makes sense since there's no recurring cadence to consider for billing alignment.
… billing cadence to be at least 1 month
Co-authored-by: Peter Turi <[email protected]>
GAlexIHU
force-pushed
the
feat/plan-billing-cadence
branch
from
8a0c51c to
d8bcbb2
Compare
Summary
This is a precusros-up of this PR: #2922
The two PRs are separated for easier reviews
They should be collapsed and merged together
Summary by CodeRabbit