chore: charges add custom currency support#4755
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| query = query.Where(dbchargecreditpurchasecreditgrant.HasCreditPurchaseWith( | ||
| dbchargecreditpurchase.HasCustomCurrencyWith( | ||
| dbcustomcurrency.CodeEQ(*input.Currency), | ||
| dbcustomcurrency.Namespace(input.Customer.Namespace), | ||
| dbcustomcurrency.DeletedAtIsNil(), | ||
| ), | ||
| dbchargecreditpurchase.FiatCurrencyCodeEQ(*input.Currency), | ||
| )) |
There was a problem hiding this comment.
HasCreditPurchaseWith combines these predicates with AND, but the new database constraint requires a charge to have either fiat_currency_code or custom_currency_id, never both. Supplying any currency filter therefore excludes the matching funded activities instead of returning them.
| query = query.Where(dbchargecreditpurchasecreditgrant.HasCreditPurchaseWith( | |
| dbchargecreditpurchase.HasCustomCurrencyWith( | |
| dbcustomcurrency.CodeEQ(*input.Currency), | |
| dbcustomcurrency.Namespace(input.Customer.Namespace), | |
| dbcustomcurrency.DeletedAtIsNil(), | |
| ), | |
| dbchargecreditpurchase.FiatCurrencyCodeEQ(*input.Currency), | |
| )) | |
| query = query.Where(dbchargecreditpurchasecreditgrant.HasCreditPurchaseWith( | |
| dbchargecreditpurchase.Or( | |
| dbchargecreditpurchase.HasCustomCurrencyWith( | |
| dbcustomcurrency.CodeEQ(*input.Currency), | |
| dbcustomcurrency.Namespace(input.Customer.Namespace), | |
| dbcustomcurrency.DeletedAtIsNil(), | |
| ), | |
| dbchargecreditpurchase.FiatCurrencyCodeEQ(*input.Currency), | |
| ), | |
| )) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/billing/charges/creditpurchase/adapter/funded_credit_activity.go
Line: 67-74
Comment:
**Mutually Exclusive Filters Are ANDed**
`HasCreditPurchaseWith` combines these predicates with AND, but the new database constraint requires a charge to have either `fiat_currency_code` or `custom_currency_id`, never both. Supplying any currency filter therefore excludes the matching funded activities instead of returning them.
```suggestion
query = query.Where(dbchargecreditpurchasecreditgrant.HasCreditPurchaseWith(
dbchargecreditpurchase.Or(
dbchargecreditpurchase.HasCustomCurrencyWith(
dbcustomcurrency.CodeEQ(*input.Currency),
dbcustomcurrency.Namespace(input.Customer.Namespace),
dbcustomcurrency.DeletedAtIsNil(),
),
dbchargecreditpurchase.FiatCurrencyCodeEQ(*input.Currency),
),
))
```
How can I resolve this? If you propose a fix, please make it concise.
| dbCustomCurrency, err := entedge.OrNilIfNotFound(edges.CustomCurrencyOrErr()) | ||
| if err != nil { | ||
| return meta.Charge{}, fmt.Errorf("failed to get custom currency: %w", err) | ||
| } | ||
|
|
||
| resolvedCurrency, err := currenciesadapter.MapCustomCurrencyOrFiatCurrencyFromDB(currenciesadapter.CustomCurrencyOrFiatCurrency{ | ||
| CustomCurrency: dbCustomCurrency, | ||
| FiatCurrency: entity.GetFiatCurrencyCode(), | ||
| }) |
There was a problem hiding this comment.
Every charge mapping now calls CustomCurrencyOrErr, including fiat charges and entities returned by update or mark-voided operations. Those write queries do not load the new edge, so the accessor returns an edge-not-loaded error and otherwise successful operations fail while mapping their result. The mapper must not require an eager-loaded edge on every path, or every caller must load it first.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/billing/charges/models/chargemeta/model.go
Line: 255-263
Comment:
**Mapping Requires an Eager-Loaded Edge**
Every charge mapping now calls `CustomCurrencyOrErr`, including fiat charges and entities returned by update or mark-voided operations. Those write queries do not load the new edge, so the accessor returns an edge-not-loaded error and otherwise successful operations fail while mapping their result. The mapper must not require an eager-loaded edge on every path, or every caller must load it first.
How can I resolve this? If you propose a fix, please make it concise.| } | ||
| if err := i.Currency.Validate(); err != nil { | ||
| errs = append(errs, fmt.Errorf("currency: %w", err)) | ||
| } else if i.Currency.Type() == currencyx.CurrencyTypeCustom { | ||
| errs = append(errs, fmt.Errorf("advance lineage backfill: %w", meta.ErrCustomCurrencyNotSupported)) | ||
| } |
There was a problem hiding this comment.
The create service now accepts and resolves a custom currency, but credit-purchase grant flows call advance-lineage backfill with that resolved currency. This new guard rejects the operation, so a custom-currency promotional or external credit purchase cannot complete its lifecycle. Either reject this currency earlier as unsupported or add lineage support before accepting the charge.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/billing/charges/lineage/service.go
Line: 114-119
Comment:
**Custom Credit Lifecycle Is Rejected**
The create service now accepts and resolves a custom currency, but credit-purchase grant flows call advance-lineage backfill with that resolved currency. This new guard rejects the operation, so a custom-currency promotional or external credit purchase cannot complete its lifecycle. Either reject this currency earlier as unsupported or add lineage support before accepting the charge.
How can I resolve this? If you propose a fix, please make it concise.| -- modify "charge_credit_purchases" table | ||
| ALTER TABLE "charge_credit_purchases" ADD CONSTRAINT "currency_reference" CHECK ((currency IS NULL) <> (custom_currency_id IS NULL)), ALTER COLUMN "currency" DROP NOT NULL, ADD COLUMN "custom_currency_id" character(26) NULL, ADD CONSTRAINT "charge_credit_purchases_custom_currencies_charges_credit_purcha" FOREIGN KEY ("custom_currency_id") REFERENCES "custom_currencies" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT; | ||
| -- modify "charge_flat_fees" table | ||
| ALTER TABLE "charge_flat_fees" ADD CONSTRAINT "currency_reference" CHECK ((currency IS NULL) <> (custom_currency_id IS NULL)), ALTER COLUMN "currency" DROP NOT NULL, ADD COLUMN "custom_currency_id" character(26) NULL, ADD CONSTRAINT "charge_flat_fees_custom_currencies_charges_flat_fee" FOREIGN KEY ("custom_currency_id") REFERENCES "custom_currencies" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT; | ||
| -- modify "charge_usage_based" table | ||
| ALTER TABLE "charge_usage_based" ADD CONSTRAINT "currency_reference" CHECK ((currency IS NULL) <> (custom_currency_id IS NULL)), ALTER COLUMN "currency" DROP NOT NULL, ADD COLUMN "custom_currency_id" character(26) NULL, ADD CONSTRAINT "charge_usage_based_custom_currencies_charges_usage_based" FOREIGN KEY ("custom_currency_id") REFERENCES "custom_currencies" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT; |
There was a problem hiding this comment.
The generated ChargesSearchV1 model now selects custom_currency_id, but this migration never recreates the existing charges_search_v1s view with that column. After upgrading an existing database, charge-search queries request a column the installed view does not expose and fail at runtime. Recreate the view in this migration after altering its source tables.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tools/migrate/migrations/20260720093016_charges-custom-currencies.up.sql
Line: 1-6
Comment:
**Search View Keeps Its Old Shape**
The generated `ChargesSearchV1` model now selects `custom_currency_id`, but this migration never recreates the existing `charges_search_v1s` view with that column. After upgrading an existing database, charge-search queries request a column the installed view does not expose and fail at runtime. Recreate the view in this migration after altering its source tables.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/openmeter/github/openmeterio/openmeter/-/custom-context?memory=eb020e3b-8e3b-45ea-a8b7-4006b2b2065b))
How can I resolve this? If you propose a fix, please make it concise.| -- reverse: modify "charge_usage_based" table | ||
| ALTER TABLE "charge_usage_based" DROP CONSTRAINT "charge_usage_based_custom_currencies_charges_usage_based", DROP COLUMN "custom_currency_id", ALTER COLUMN "currency" SET NOT NULL, DROP CONSTRAINT "currency_reference"; | ||
| -- reverse: modify "charge_flat_fees" table | ||
| ALTER TABLE "charge_flat_fees" DROP CONSTRAINT "charge_flat_fees_custom_currencies_charges_flat_fee", DROP COLUMN "custom_currency_id", ALTER COLUMN "currency" SET NOT NULL, DROP CONSTRAINT "currency_reference"; | ||
| -- reverse: modify "charge_credit_purchases" table | ||
| ALTER TABLE "charge_credit_purchases" DROP CONSTRAINT "charge_credit_purchases_custom_currencies_charges_credit_purcha", DROP COLUMN "custom_currency_id", ALTER COLUMN "currency" SET NOT NULL, DROP CONSTRAINT "currency_reference"; |
There was a problem hiding this comment.
A custom-currency charge has currency IS NULL and only custom_currency_id populated. This rollback drops that ID and then restores currency to NOT NULL, so it fails as soon as any custom-currency charge exists. The rollback needs an explicit conversion or deletion policy before restoring the old constraint.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tools/migrate/migrations/20260720093016_charges-custom-currencies.down.sql
Line: 1-6
Comment:
**Rollback Cannot Convert Custom Charges**
A custom-currency charge has `currency IS NULL` and only `custom_currency_id` populated. This rollback drops that ID and then restores `currency` to NOT NULL, so it fails as soon as any custom-currency charge exists. The rollback needs an explicit conversion or deletion policy before restoring the old constraint.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/openmeter/github/openmeterio/openmeter/-/custom-context?memory=eb020e3b-8e3b-45ea-a8b7-4006b2b2065b))
How can I resolve this? If you propose a fix, please make it concise.1 participant
Overview
Fixes #(issue)
Notes for reviewer
Greptile Summary
This PR adds custom-currency support to billing charges. The main changes are:
Confidence Score: 4/5
Credit-purchase creation, currency-filtered activity queries, and database upgrades can fail on the updated paths.
Credit-purchase adapters and lineage handling, shared charge mapping, and both custom-currency migration files.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Credit purchase request] --> B[Resolve currency by namespace and code] B --> C[Normalize amount precision] C --> D[Persist charge] D --> E{Currency type} E -->|Fiat| F[Store currency code] E -->|Custom| G[Store custom currency ID] F --> H[Map charge with resolved currency] G --> H H --> I[Funded activity and lineage flows]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Credit purchase request] --> B[Resolve currency by namespace and code] B --> C[Normalize amount precision] C --> D[Persist charge] D --> E{Currency type} E -->|Fiat| F[Store currency code] E -->|Custom| G[Store custom currency ID] F --> H[Map charge with resolved currency] G --> H H --> I[Funded activity and lineage flows]Comments Outside Diff (1)
openmeter/billing/charges/creditpurchase/adapter/charge.go, line 84-89 (link)service.Createresolves the requested currency, but only uses it for amount normalization before calling this adapter. This newchargemeta.CreateInputrequiresCurrency, yet the value is not carried throughcreditpurchase.CreateInputor set here, so every valid credit-purchase create returnscurrency is requiredbefore persistence.Context Used: AGENTS.md (source)
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore: charges add custom currency suppo..." | Re-trigger Greptile
Context used: