feat(customcurrency): support custom currency subscriptions - #4744
Open
GAlexIHU wants to merge 10 commits into
Open
feat(customcurrency): support custom currency subscriptions#4744GAlexIHU wants to merge 10 commits into
GAlexIHU wants to merge 10 commits into
Conversation
Contributor
|
Too many files changed for review (155 files, 100 file limit). Bypass the limit by tagging |
Contributor
📝 WalkthroughWalkthroughThis change adds custom-currency subscription support. It adds invoice-currency and cost-basis fields, persists dynamic or pinned cost bases, resolves currencies during subscription workflows, skips unsupported billing synchronization, updates API models, and adds migration and integration coverage. ChangesCustom-currency subscriptions
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SubscriptionAPI
participant SubscriptionWorkflow
participant CurrencyResolver
participant SubscriptionRepository
participant BillingSync
Client->>SubscriptionAPI: create or change subscription with cost-basis mode
SubscriptionAPI->>SubscriptionWorkflow: pass invoice currency and cost-basis mode
SubscriptionWorkflow->>CurrencyResolver: resolve currency and effective cost basis
CurrencyResolver-->>SubscriptionWorkflow: return currency and cost-basis data
SubscriptionWorkflow->>SubscriptionRepository: persist subscription and pinned cost bases
SubscriptionRepository-->>SubscriptionAPI: return hydrated subscription
SubscriptionAPI-->>Client: return subscription with invoice currency and cost-basis pins
BillingSync->>SubscriptionWorkflow: synchronize subscription events
SubscriptionWorkflow-->>BillingSync: skip custom-currency billing or return conflict
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GAlexIHU
force-pushed
the
codex/custom-currency-subscription-semantics
branch
from
July 17, 2026 21:08
d6b8deb to
8e4110f
Compare
chrisgacsal
force-pushed
the
codex/custom-currency-subscriptions
branch
4 times, most recently
from
August 6, 2026 12:42
8f0d177 to
5d2efe2
Compare
chrisgacsal
force-pushed
the
codex/custom-currency-subscription-semantics
branch
from
August 6, 2026 16:15
8e4110f to
b0af507
Compare
chrisgacsal
force-pushed
the
codex/custom-currency-subscription-semantics
branch
from
August 12, 2026 09:19
b0af507 to
604e6fd
Compare
chrisgacsal
force-pushed
the
codex/custom-currency-subscription-semantics
branch
from
August 12, 2026 13:57
604e6fd to
edb984d
Compare
chrisgacsal
marked this pull request as ready for review
August 12, 2026 14:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
openmeter/customer/customer.go (1)
176-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the fiat-only customer-currency rule.
validateCustomerCurrencyrejects custom currencies. Add a doc comment that states customer billing currencies must be fiat and explains the subscription invoice-currency constraint.As per coding guidelines, “Document domain helpers whose names compress important business semantics, including observable behavior and why excluded cases are excluded.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openmeter/customer/customer.go` around lines 176 - 190, Add a doc comment for validateCustomerCurrency stating that customer billing currencies must be fiat and explaining that subscription invoices require a fiat currency, excluding custom currencies. Keep the validation behavior unchanged.Source: Coding guidelines
openmeter/subscription/addon/diff/apply.go (1)
123-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the currency validation into a named helper.
getApplyForRateCardmust use a callback forsubscription.NewAppliesToSpec, but this currency compatibility rule is meaningful domain validation. Move the check into a helper such asvalidateAddonCurrencyCompatibilityand call it from the callback. This keeps the rule testable and avoids hiding validation in a local closure.As per coding guidelines, “Do not hide type switching, validation, persistence mapping, or meaningful domain translation inside local closures; use named helpers and reserve inline callbacks for obvious, tiny logic.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openmeter/subscription/addon/diff/apply.go` around lines 123 - 129, The currency compatibility check currently embedded in the NewAppliesToSpec callback should be extracted into a named helper such as validateAddonCurrencyCompatibility. Update getApplyForRateCard to invoke that helper from the callback, preserving the existing nil handling, effective-currency comparison, and ErrPlanAddonCurrencyMismatch error behavior.Source: Coding guidelines
test/subscription/custom_currency_test.go (2)
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a fixed date instead of
time.Now()for the frozen clock.The test anchors everything on wall-clock time. With
BillingCadenceofP1MandstartsAt = now + 1h, a run near a month or DST boundary can behave differently from a run mid-month, and a failure is then hard to reproduce. Other new tests in this stack pin a fixed instant, for exampletime.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC).♻️ Suggested change
- now := time.Now().UTC().Truncate(time.Second) + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) clock.FreezeTime(now) defer clock.UnFreeze()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/subscription/custom_currency_test.go` around lines 37 - 39, Replace the time.Now().UTC() anchor in the test’s frozen-clock setup with a fixed time.Date instant in UTC, while preserving the existing truncation, FreezeTime call, and deferred UnFreeze cleanup.
146-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSmall doc gap: the
thenblock only covers the rejection case.The subtest name promises dynamic and pinned resolution, and the body asserts both (no pin for dynamic, one pin with the exact cost basis for pinned). The
thencomment stops at the rejection. Extending it keeps the intent block matching the assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/subscription/custom_currency_test.go` around lines 146 - 216, The then comment in the “credit then invoice resolves dynamic and pinned cost bases” subtest should also describe the successful dynamic and pinned outcomes: dynamic mode resolves without persisting a pin, while pinned mode stores the selected cost basis pin. Keep the existing rejection expectation and align the intent comments with the assertions in this test.openmeter/subscription/apply.go (1)
60-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNice fix on the shadowing. One small idea: keep the original error too.
When
models.AsValidationIssuesfails, onlyconversionErrpropagates. The original apply error, which is the actual cause, is dropped. Joining both keeps the failure traceable.♻️ Optional tweak
issues, conversionErr := models.AsValidationIssues(err) if conversionErr != nil { - return wrapError(conversionErr) + return wrapError(errors.Join(err, conversionErr)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openmeter/subscription/apply.go` around lines 60 - 63, Update the error branch after models.AsValidationIssues in the apply flow to return an error that combines conversionErr with the original err, preserving both the conversion failure and the underlying apply error while retaining the existing wrapping behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/spec/packages/aip-client-javascript/src/index.ts`:
- Line 136: Update the generator or template that produces the SDK root exports
so SubscriptionCostBasisMode is included alongside SubscriptionCostBasisPin,
then regenerate the generated index.ts file. Do not modify the generated file
manually.
In `@api/v3/handlers/subscriptions/change.go`:
- Line 118: Preserve an omitted CostBasisMode throughout the subscription-change
workflow instead of converting nil to an empty value. In
api/v3/handlers/subscriptions/change.go:118-118, retain curr.CostBasisMode when
body.CostBasisMode is nil; in
openmeter/productcatalog/subscription/http/change.go:93-93 and :131-131, keep
the field omitted for custom-plan and referenced-plan changes so the existing
subscription mode remains effective.
In `@docs/migration-guides/2026-07-17-custom-currency-subscriptions.md`:
- Line 5: Update the migration guide filename/date to 2026-08-10 and replace the
nonexistent migration reference 20260717195001 with
20260810084018_custom_currency_subscription_semantics.
In `@openmeter/productcatalog/subscription/service/create_test.go`:
- Around line 231-241: Align the `then` comments in
`TestCreateInlineCreditOnlyPlanSkipsCurrencyCostBasis` with the asserted
successful creation behavior: document that validation skips the missing
custom-currency cost basis for the credit-only plan and returns no error. Update
the corresponding repeated comments in the later assertion block as well.
In `@openmeter/productcatalog/subscription/service/service.go`:
- Around line 58-59: Update Config.Validate to wrap the joined field errors with
models.NewNillableGenericValidationError before returning, preserving the
generic validation classification and existing field context; keep New’s
handling unchanged.
In `@openmeter/subscription/repo/subscriptionrepo.go`:
- Around line 163-194: Move all field validation from the transaction callback
into CreateCostBasisPinEntityInput.Validate, collecting namespace, subscription
ID, custom currency ID, cost basis ID, and invoice currency errors and returning
models.NewNillableGenericValidationError(errors.Join(errs...)). Invoke Validate
on the inputs before opening the transaction, leaving the callback responsible
only for building and persisting SubscriptionCostBasisPin entities.
In `@openmeter/subscription/testutils/mock.go`:
- Around line 44-45: Update MockService.Update and the corresponding UpdateFn
signature to accept variadic subscription.UpdateOption values, then forward
options when invoking UpdateFn so mock-backed workflows preserve
subscription.WithCostBasisEffectiveAt and other update options.
In `@tools/migrate/subscription_custom_currency_semantics_test.go`:
- Around line 162-166: Extend the rollback test after Migrate(previousVersion)
to set currency = NULL for the existing priced itemID and require the update
succeeds, confirming the restored subscription_item_currency_has_price
constraint is active. Keep the existing assertion that
subscription_cost_basis_pins is removed.
---
Nitpick comments:
In `@openmeter/customer/customer.go`:
- Around line 176-190: Add a doc comment for validateCustomerCurrency stating
that customer billing currencies must be fiat and explaining that subscription
invoices require a fiat currency, excluding custom currencies. Keep the
validation behavior unchanged.
In `@openmeter/subscription/addon/diff/apply.go`:
- Around line 123-129: The currency compatibility check currently embedded in
the NewAppliesToSpec callback should be extracted into a named helper such as
validateAddonCurrencyCompatibility. Update getApplyForRateCard to invoke that
helper from the callback, preserving the existing nil handling,
effective-currency comparison, and ErrPlanAddonCurrencyMismatch error behavior.
In `@openmeter/subscription/apply.go`:
- Around line 60-63: Update the error branch after models.AsValidationIssues in
the apply flow to return an error that combines conversionErr with the original
err, preserving both the conversion failure and the underlying apply error while
retaining the existing wrapping behavior.
In `@test/subscription/custom_currency_test.go`:
- Around line 37-39: Replace the time.Now().UTC() anchor in the test’s
frozen-clock setup with a fixed time.Date instant in UTC, while preserving the
existing truncation, FreezeTime call, and deferred UnFreeze cleanup.
- Around line 146-216: The then comment in the “credit then invoice resolves
dynamic and pinned cost bases” subtest should also describe the successful
dynamic and pinned outcomes: dynamic mode resolves without persisting a pin,
while pinned mode stores the selected cost basis pin. Keep the existing
rejection expectation and align the intent comments with the assertions in this
test.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: de7706e2-9e1b-429a-afc2-40d429da40a0
⛔ Files ignored due to path filters (50)
api/client/go/client.gen.gois excluded by!api/client/**api/client/javascript/src/client/schemas.tsis excluded by!api/client/**api/client/javascript/src/zod/index.tsis excluded by!api/client/**api/client/python/openmeter/_generated/models/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/types.pyis excluded by!**/_generated/**,!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yamlapi/v3/openapi.yamlis excluded by!**/openapi.yamlgo.sumis excluded by!**/*.sum,!**/*.sumopenmeter/ent/db/client.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/where.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_create.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_query.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_update.gois excluded by!**/ent/db/**openmeter/ent/db/cursor.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency/customcurrency.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency/where.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_create.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_query.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_update.gois excluded by!**/ent/db/**openmeter/ent/db/ent.gois excluded by!**/ent/db/**openmeter/ent/db/entmixinaccessor.gois excluded by!**/ent/db/**openmeter/ent/db/expose.gois excluded by!**/ent/db/**openmeter/ent/db/hook/hook.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**openmeter/ent/db/paginate.gois excluded by!**/ent/db/**openmeter/ent/db/predicate/predicate.gois excluded by!**/ent/db/**openmeter/ent/db/runtime.gois excluded by!**/ent/db/**openmeter/ent/db/setorclear.gois excluded by!**/ent/db/**openmeter/ent/db/subscription.gois excluded by!**/ent/db/**openmeter/ent/db/subscription/subscription.gois excluded by!**/ent/db/**openmeter/ent/db/subscription/where.gois excluded by!**/ent/db/**openmeter/ent/db/subscription_create.gois excluded by!**/ent/db/**openmeter/ent/db/subscription_query.gois excluded by!**/ent/db/**openmeter/ent/db/subscription_update.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin/subscriptioncostbasispin.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin/where.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin_create.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin_delete.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin_query.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptioncostbasispin_update.gois excluded by!**/ent/db/**openmeter/ent/db/tx.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (100)
api/api.gen.goapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/models/operations/subscriptions.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/subscriptions/subscription.tspapi/spec/packages/legacy/src/productcatalog/subscription.tspapi/spec/packages/legacy/src/types.tspapi/v3/api.gen.goapi/v3/client/models_subscriptions.goapi/v3/handlers/subscriptions/change.goapi/v3/handlers/subscriptions/convert.goapi/v3/handlers/subscriptions/convert_test.goapp/common/subscription.gocmd/billing-worker/wire_gen.gocmd/jobs/internal/wire_gen.gocmd/server/wire_gen.godocs/migration-guides/2026-07-17-custom-currency-subscriptions.mdopenmeter/billing/worker/subscriptionsync/reconciler/reconciler.goopenmeter/billing/worker/subscriptionsync/reconciler/reconciler_test.goopenmeter/billing/worker/subscriptionsync/service.goopenmeter/billing/worker/subscriptionsync/service/base_test.goopenmeter/billing/worker/subscriptionsync/service/currency_boundary_test.goopenmeter/billing/worker/subscriptionsync/service/handlers.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.goopenmeter/billing/worker/subscriptionsync/service/service.goopenmeter/billing/worker/subscriptionsync/service/sync.goopenmeter/billing/worker/subscriptionsync/service/targetstate/targetstate.goopenmeter/billing/worker/worker.goopenmeter/customer/customer.goopenmeter/customer/customer_test.goopenmeter/ent/schema/custom_currencies.goopenmeter/ent/schema/subscription.goopenmeter/ent/schema/subscription_cost_basis_pin.goopenmeter/productcatalog/subscription/http/change.goopenmeter/productcatalog/subscription/http/create.goopenmeter/productcatalog/subscription/http/mapping.goopenmeter/productcatalog/subscription/http/mapping_test.goopenmeter/productcatalog/subscription/service/change.goopenmeter/productcatalog/subscription/service/change_test.goopenmeter/productcatalog/subscription/service/create.goopenmeter/productcatalog/subscription/service/create_test.goopenmeter/productcatalog/subscription/service/migrate.goopenmeter/productcatalog/subscription/service/migrate_test.goopenmeter/productcatalog/subscription/service/plan_test.goopenmeter/productcatalog/subscription/service/service.goopenmeter/server/server_test.goopenmeter/subscription/addon/diff/apply.goopenmeter/subscription/addon/diff/apply_test.goopenmeter/subscription/addon/extend.goopenmeter/subscription/addon/service/change_test.goopenmeter/subscription/addon/service/create_test.goopenmeter/subscription/addon/service/currency_test.goopenmeter/subscription/addon/service/list_test.goopenmeter/subscription/apply.goopenmeter/subscription/currency.goopenmeter/subscription/currency_validation_test.goopenmeter/subscription/errors_test.goopenmeter/subscription/events_test.goopenmeter/subscription/patch/patch_test.goopenmeter/subscription/repo/mapping.goopenmeter/subscription/repo/subscriptionitemrepo_test.goopenmeter/subscription/repo/subscriptionrepo.goopenmeter/subscription/repo/subscriptionrepo_test.goopenmeter/subscription/repository.goopenmeter/subscription/service.goopenmeter/subscription/service/cost_basis_persistence_test.goopenmeter/subscription/service/currency.goopenmeter/subscription/service/currency_materialization_test.goopenmeter/subscription/service/currency_test.goopenmeter/subscription/service/service.goopenmeter/subscription/service/service_test.goopenmeter/subscription/service/servicevalidation.goopenmeter/subscription/service/servicevalidation_test.goopenmeter/subscription/service/sync_test.goopenmeter/subscription/subscription.goopenmeter/subscription/subscriptionspec.goopenmeter/subscription/subscriptionview.goopenmeter/subscription/testutils/builder.goopenmeter/subscription/testutils/compare.goopenmeter/subscription/testutils/mock.goopenmeter/subscription/testutils/repository.goopenmeter/subscription/testutils/service.goopenmeter/subscription/uniqueness_test.goopenmeter/subscription/workflow/service.goopenmeter/subscription/workflow/service/addon.goopenmeter/subscription/workflow/service/addon_test.goopenmeter/subscription/workflow/service/currency_test.goopenmeter/subscription/workflow/service/service.goopenmeter/subscription/workflow/service/service_test.goopenmeter/subscription/workflow/service/subscription.goopenmeter/subscription/workflow/service/subscription_test.gotest/billing/subscription_suite.gotest/customer/customer.gotest/customer/testenv.gotest/subscription/custom_currency_test.gotest/subscription/framework_test.gotools/migrate/migrations/20260810084018_custom_currency_subscription_semantics.down.sqltools/migrate/migrations/20260810084018_custom_currency_subscription_semantics.up.sqltools/migrate/subscription_custom_currency_semantics_test.go
💤 Files with no reviewable changes (1)
- openmeter/subscription/service/currency_materialization_test.go
Contributor
Author
|
👍 ✅ (apart from CC addons being blocked) |
turip
reviewed
Aug 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/migration-guides/2026-08-10-custom-currency-subscriptions.md`:
- Around line 3-6: Update the migration guide to remove the rollout step and
description for migration 20260810064730_backfill_subscription_item_currencies.
Document only the supported preflight and recovery process, without stating that
legacy priced items are automatically backfilled before the currency constraint
migration.
In `@openmeter/subscription/repository_test.go`:
- Around line 22-79: Extend the validation test table around
CreateCostBasisPinEntityInput validation with a case that clears two required
fields, such as Namespace and SubscriptionID, and expects an error containing
both corresponding validation messages. Ensure the assertion verifies collection
of both errors rather than only the first returned message, preserving the
existing single-field cases.
In `@openmeter/subscription/repository.go`:
- Around line 94-95: Update the invoice currency validation in the surrounding
validation method: split the i.InvoiceCurrency.Validate() and IsFiat() checks so
validation errors are handled separately. Wrap and retain the Validate() error
with invoice-currency context, while continuing to report a distinct
invalid-currency error for non-fiat values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a4a0d879-1d6c-491f-aea6-1f10679e08b8
📒 Files selected for processing (15)
docs/migration-guides/2026-08-10-custom-currency-subscriptions.mdopenmeter/customer/customer.goopenmeter/productcatalog/subscription/service/create_test.goopenmeter/productcatalog/subscription/service/service.goopenmeter/subscription/addon/diff/apply.goopenmeter/subscription/addon/service/currency_test.goopenmeter/subscription/addon/service/service.goopenmeter/subscription/errors.goopenmeter/subscription/repo/subscriptionrepo.goopenmeter/subscription/repository.goopenmeter/subscription/repository_test.goopenmeter/subscription/testutils/mock.goopenmeter/subscription/testutils/mock_test.goopenmeter/subscription/workflow/service/subscription_test.gotools/migrate/subscription_custom_currency_semantics_test.go
💤 Files with no reviewable changes (2)
- openmeter/subscription/errors.go
- openmeter/subscription/addon/service/service.go
🚧 Files skipped from review as they are similar to previous changes (5)
- openmeter/customer/customer.go
- openmeter/subscription/workflow/service/subscription_test.go
- tools/migrate/subscription_custom_currency_semantics_test.go
- openmeter/productcatalog/subscription/service/service.go
- openmeter/subscription/repo/subscriptionrepo.go
chrisgacsal
force-pushed
the
codex/custom-currency-subscription-semantics
branch
from
August 12, 2026 19:28
3f9a303 to
4e3ae0e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/custom_currencies_productcatalog_v3_test.go`:
- Line 254: Update the status assertion in the affected test to expect
http.StatusOK, matching the success status returned by the subscription addon
creation endpoint in create.go; leave the request and error handling unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1431fb2a-0cc8-4d90-b610-d3c2173d300d
📒 Files selected for processing (3)
e2e/custom_currencies_productcatalog_v3_test.goopenmeter/subscription/repository.goopenmeter/subscription/repository_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- openmeter/subscription/repository_test.go
- openmeter/subscription/repository.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Add end-to-end subscription-domain support for custom-currency priced items:
InvoiceCurrency, while preserving the legacy currency JSON field.Why
Product catalog plans and add-ons can now use custom currencies, but subscriptions previously assumed that every priced item used the subscription’s fiat currency. That caused custom-currency identity to be lost when catalog resources were materialized into subscriptions.
The change separates two concepts:
A custom-currency item must eventually be converted into the subscription’s invoice currency. Cost-basis modes define whether that conversion rate follows the effective cost basis dynamically or is fixed when the currency pair is introduced.
The billing guard prevents the subscription rollout from accidentally generating incorrect fiat invoice lines before downstream custom-currency billing is fully supported.
How
The workflow determines the invoice currency from the customer and plan:
Before validation and persistence, the subscription service:
For
credit_then_invoice, the service verifies that every custom-currency item has an effective cost basis into the invoice currency:Persistence adds:
cost_basis_modeto subscriptions.subscription_cost_basis_pinstable.The migration intentionally does not backfill legacy priced items. Existing installations must populate their inherited currency before applying the new constraint.
API schemas and generated Go, JavaScript, and Python clients are regenerated to expose the new fields.
Billing synchronization detects custom-currency billables before constructing fiat billing targets, preventing partial or incorrect reconciliation until billing support is added.
Summary by CodeRabbit