feat(currencies): add custom currency update endpoint - #4922
feat(currencies): add custom currency update endpoint#4922borbelyr-kong wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds custom currency update support through a PUT endpoint. It defines update models and validation, updates currency service and persistence layers, exposes Go and JavaScript client methods, wires the HTTP route, and adds service and end-to-end tests. ChangesCustom currency update
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This change adds editing for custom currency presentation fields without supplied evidence of a current correctness or production risk; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant GoClient
participant Server
participant UpdateCurrencyHandler
participant CurrencyService
participant CurrencyRepository
GoClient->>Server: PUT custom currency update
Server->>UpdateCurrencyHandler: Forward currency ID and request
UpdateCurrencyHandler->>CurrencyService: UpdateCurrency with namespaced input
CurrencyService->>CurrencyRepository: Update mutable presentation fields
CurrencyRepository-->>CurrencyService: Return updated currency
CurrencyService-->>UpdateCurrencyHandler: Return updated currency
UpdateCurrencyHandler-->>Server: Encode HTTP 200 JSON response
Server-->>GoClient: Return CurrencyCustom response
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/v3/handlers/currencies/update.go (1)
27-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the non-trivial callbacks into named methods.
These callbacks contain meaningful validation, domain translation, persistence, and error mapping. Keep the transaction and transport callbacks small. Delegate the work to named methods.
api/v3/handlers/currencies/update.go#L27-L61: Extract request parsing and service-response mapping into named handler methods.openmeter/currencies/adapter/currencies.go#L222-L245: Extract the Ent update and DB error mapping into a named adapter method.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 `@api/v3/handlers/currencies/update.go` around lines 27 - 61, Extract the non-trivial transport callbacks in api/v3/handlers/currencies/update.go lines 27-61 into named handler methods: move request validation, namespace resolution, body parsing, and UpdateCurrencyRequest construction into one method, and move service response mapping into another, leaving only small delegating callbacks. In openmeter/currencies/adapter/currencies.go lines 222-245, extract the Ent update and database error mapping into a named adapter method, keeping the surrounding transaction callback as a thin delegate.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@api/v3/handlers/currencies/update.go`:
- Around line 27-61: Extract the non-trivial transport callbacks in
api/v3/handlers/currencies/update.go lines 27-61 into named handler methods:
move request validation, namespace resolution, body parsing, and
UpdateCurrencyRequest construction into one method, and move service response
mapping into another, leaving only small delegating callbacks. In
openmeter/currencies/adapter/currencies.go lines 222-245, extract the Ent update
and database error mapping into a named adapter method, keeping the surrounding
transaction callback as a thin delegate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ec02233-6bb8-4a7f-a0cb-83827fbd38d9
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (24)
api/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/currencies.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/models/operations/currencies.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/src/sdk/internal.tsapi/spec/packages/aip/src/currencies/currency.tspapi/spec/packages/aip/src/currencies/operations.tspapi/v3/api.gen.goapi/v3/client/README.mdapi/v3/client/currencies.goapi/v3/client/models_currencies.goapi/v3/handlers/currencies/handler.goapi/v3/handlers/currencies/handler_test.goapi/v3/handlers/currencies/update.goapi/v3/server/routes.goe2e/currencies_v3_test.goopenmeter/currencies/adapter/currencies.goopenmeter/currencies/repository.goopenmeter/currencies/service.goopenmeter/currencies/service/service.goopenmeter/currencies/service/service_test.goopenmeter/server/server_test.go
c36c5f3 to
7639a20
Compare
7639a20 to
4cdf9b0
Compare
What
Adds
PUT /api/v3/openmeter/currencies/custom/{currencyId}(update-custom-currency) so users can edit a custom currency's presentational attributes: name, symbol, decimal mark and thousand separator. Custom currencies were previously create/read-only.Why
Users who typo a currency's name or want to change how amounts are formatted currently have no way to fix it — the only workaround is creating a replacement currency, which is not viable once charges or plans reference the original.
codeandprecisionstay immutable, and are absent from the request model rather than merely rejected. Charges, invoices and ledger currency dimensions are denominated with them, andCurrencyReference.MarshalTextsnapshots the precision into persisted ledger references — editing either would misstate historical monetary records.How
api/spec/packages/aip/src/currencies/): newCurrencyCustomUpdatemodel plus a@putoperation on the existingCurrenciesCustomOperationsinterface. A dedicated request model (rather thanShared.UpdateRequest<CurrencyCustom>) keeps the immutable fields unrepresentable and leaves the sharedCurrencyBasevisibility untouched — the generated diff is additive only.openmeter/currencies/):UpdateCurrencyInputwithValidate(), threaded through the service and repository interfaces.UpdateOneIDscoped by namespace andDeletedAtIsNil(); not-found maps to 404. No read-then-merge is needed because the request carries every mutable attribute.api/v3/handlers/currencies/update.go): sits behind the samecreditsEnabledgate as create, and usesapierrors.GenericErrorEncoder().Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Greptile Summary
The PR adds a namespace-scoped PUT endpoint for replacing a custom currency’s mutable presentation attributes while preserving its code and precision.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant API as V3 API participant Handler as Currency Handler participant Service as Currency Service participant DB as Ent/PostgreSQL Client->>API: "PUT /currencies/custom/{currencyId}" API->>API: Validate request against OpenAPI API->>Handler: Dispatch with namespace and currency ID Handler->>Service: UpdateCurrency(input) Service->>Service: Validate mutable attributes Service->>DB: Update ID scoped by namespace and deleted_at IS NULL DB-->>Service: Updated custom currency Service-->>Handler: Currency Handler-->>Client: 200 BillingCurrencyCustomReviews (7): Last reviewed commit: "refactor(currencies): remove unnecessary..." | Re-trigger Greptile
Context used: