feat(customcurrency): materialize subscription item currencies#4742
Open
GAlexIHU wants to merge 1 commit into
Open
feat(customcurrency): materialize subscription item currencies#4742GAlexIHU wants to merge 1 commit into
GAlexIHU wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughSubscription items now persist fiat or custom currency references, materialize effective currency identities across subscription and addon flows, reconstruct currencies from storage, and validate workflow service dependencies during initialization. ChangesSubscription currency support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+170
to
+175
| currencyRef, err := currencyadapter.ToDBCurrencyReference(input.RateCard.AsMeta().Currency, true) | ||
| if err != nil { | ||
| return def, fmt.Errorf("invalid subscription item currency: %w", err) | ||
| } | ||
| cmd.SetNillableFiatCurrencyCode(currencyRef.FiatCurrencyCode) | ||
| cmd.SetNillableCustomCurrencyID(currencyRef.CustomCurrencyID) |
Contributor
There was a problem hiding this comment.
When a managed currency from another namespace reaches this repository, the code persists its ID without checking it against input.Namespace. The new foreign key only covers custom_currency_id, and hydration also performs no namespace check, so the item can load and use another tenant's currency instead of rejecting the write.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/subscription/repo/subscriptionitemrepo.go
Line: 170-175
Comment:
**Cross-Namespace Currency Reference**
When a managed currency from another namespace reaches this repository, the code persists its ID without checking it against `input.Namespace`. The new foreign key only covers `custom_currency_id`, and hydration also performs no namespace check, so the item can load and use another tenant's currency instead of rejecting the write.
How can I resolve this? If you propose a fix, please make it concise.
GAlexIHU
marked this pull request as ready for review
July 17, 2026 17:45
![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: 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 `@openmeter/ent/schema/subscription.go`:
- Around line 273-277: Make custom-currency relationships tenant-scoped end to
end: update the subscription schema edge to use namespace and currency ID,
regenerate the migration and extend its database test to reject cross-namespace
currencies; update the mapping hydration path to reject namespace mismatches and
the subscription item save path to validate managed-currency ownership, adding
repository coverage for cross-namespace identities. Apply these changes at
openmeter/ent/schema/subscription.go:273-277,
tools/migrate/migrations/20260717150347_add_subscription_item_currency_references.up.sql:2,
tools/migrate/subscription_item_currency_references_test.go:133-149,
openmeter/subscription/repo/mapping.go:131-135,
openmeter/subscription/repo/subscriptionitemrepo.go:170-175, and
openmeter/subscription/repo/subscriptionitemrepo_test.go:65-129.
🪄 Autofix (Beta)
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
Run ID: 6b6f6390-f688-448a-9c63-065d54815580
⛔ Files ignored due to path filters (18)
openmeter/ent/db/client.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/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.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/subscriptionitem.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptionitem/subscriptionitem.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptionitem/where.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptionitem_create.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptionitem_query.gois excluded by!**/ent/db/**openmeter/ent/db/subscriptionitem_update.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (25)
app/common/subscription.goopenmeter/ent/schema/custom_currencies.goopenmeter/ent/schema/subscription.goopenmeter/productcatalog/subscription/plan.goopenmeter/subscription/addon/diff/apply.goopenmeter/subscription/addon/diff/apply_test.goopenmeter/subscription/plan.goopenmeter/subscription/repo/mapping.goopenmeter/subscription/repo/subscriptionitemrepo.goopenmeter/subscription/repo/subscriptionitemrepo_test.goopenmeter/subscription/service/currency_materialization_test.goopenmeter/subscription/service/service.goopenmeter/subscription/service/sync.goopenmeter/subscription/subscriptionspec.goopenmeter/subscription/testutils/service.goopenmeter/subscription/workflow/service/addon_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/testenv.gotools/migrate/migrations/20260717150347_add_subscription_item_currency_references.down.sqltools/migrate/migrations/20260717150347_add_subscription_item_currency_references.up.sqltools/migrate/subscription_item_currency_references_test.go
Comment on lines
+273
to
+277
| edge.From("custom_currency", CustomCurrency.Type). | ||
| Ref("subscription_items"). | ||
| Field("custom_currency_id"). | ||
| Unique(). | ||
| Immutable(), |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Managed currency references are not namespace-scoped.
The schema, persistence, and hydration paths all accept a custom-currency ID without proving it belongs to the subscription item’s namespace.
openmeter/ent/schema/subscription.go#L273-L277: model the relationship using both namespace and currency ID.tools/migrate/migrations/20260717150347_add_subscription_item_currency_references.up.sql#L2-L2: regenerate the migration with the tenant-scoped foreign key.tools/migrate/subscription_item_currency_references_test.go#L133-L149: verify the database rejects a currency from another namespace.openmeter/subscription/repo/mapping.go#L131-L135: reject loaded custom currencies whose namespace differs from the item.openmeter/subscription/repo/subscriptionitemrepo.go#L170-L175: validate managed-currency ownership before saving.openmeter/subscription/repo/subscriptionitemrepo_test.go#L65-L129: cover repository rejection of cross-namespace identities.
📍 Affects 6 files
openmeter/ent/schema/subscription.go#L273-L277(this comment)tools/migrate/migrations/20260717150347_add_subscription_item_currency_references.up.sql#L2-L2tools/migrate/subscription_item_currency_references_test.go#L133-L149openmeter/subscription/repo/mapping.go#L131-L135openmeter/subscription/repo/subscriptionitemrepo.go#L170-L175openmeter/subscription/repo/subscriptionitemrepo_test.go#L65-L129
🤖 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/ent/schema/subscription.go` around lines 273 - 277, Make
custom-currency relationships tenant-scoped end to end: update the subscription
schema edge to use namespace and currency ID, regenerate the migration and
extend its database test to reject cross-namespace currencies; update the
mapping hydration path to reject namespace mismatches and the subscription item
save path to validate managed-currency ownership, adding repository coverage for
cross-namespace identities. Apply these changes at
openmeter/ent/schema/subscription.go:273-277,
tools/migrate/migrations/20260717150347_add_subscription_item_currency_references.up.sql:2,
tools/migrate/subscription_item_currency_references_test.go:133-149,
openmeter/subscription/repo/mapping.go:131-135,
openmeter/subscription/repo/subscriptionitemrepo.go:170-175, and
openmeter/subscription/repo/subscriptionitemrepo_test.go:65-129.
Sources: Coding guidelines, Path instructions
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
1 participant
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.
Summary
Rollout boundary
This is the expand-and-write breakpoint for subscription custom currencies.
Subscription.Currencykeeps its current meaningDeployed environments can backfill existing rows after this version is rolled out. The semantic change can then land separately.
Validation
make generatemake lint-goPOSTGRES_HOST=127.0.0.1 go test -tags=dynamic ./openmeter/subscription/... -count=1test/notificationfailure because Svix was not running on127.0.0.1:8071Greptile Summary
This PR materializes effective currencies on subscription items. The main changes are:
Confidence Score: 4/5
The custom-currency write path needs tenant ownership validation before merging.
openmeter/subscription/repo/subscriptionitemrepo.go and the subscription-item custom-currency schema relationship
Security Review
The new custom-currency reference is not scoped to the subscription item's namespace. A cross-namespace managed currency can therefore be persisted and hydrated unless the repository validates tenant ownership before writing.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Plan or add-on rate card] --> B[Subscription item specification] B --> C{Priced item has currency?} C -- No --> D[Materialize effective currency] C -- Yes --> E[Preserve currency identity] D --> F[Subscription item repository] E --> F F --> G{Currency type} G -- Fiat --> H[Store currency code] G -- Managed custom --> I[Store custom currency ID] H --> J[Subscription item row] I --> J J --> K[Eager-load and hydrate identity]%%{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[Plan or add-on rate card] --> B[Subscription item specification] B --> C{Priced item has currency?} C -- No --> D[Materialize effective currency] C -- Yes --> E[Preserve currency identity] D --> F[Subscription item repository] E --> F F --> G{Currency type} G -- Fiat --> H[Store currency code] G -- Managed custom --> I[Store custom currency ID] H --> J[Subscription item row] I --> J J --> K[Eager-load and hydrate identity]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(customcurrency): materialize subscr..." | Re-trigger Greptile
Context used (3)
Summary by CodeRabbit
New Features
Bug Fixes