refactor: currencies service pkg

[chrisgacsal]

Overview

Refactor currencies service package to be more aligned with the project. No functional changes.

Summary by CodeRabbit

• New Features
  • Expanded currency support to include cost-basis records with effective date ranges and rate handling, alongside existing custom/fiat currencies.
  • Enhanced listing behavior with request validation, sorting, and filtering options for currencies and cost bases.
• Bug Fixes
  • Standardized currency handler/service wiring so all currency operations follow the same consistent execution path.
• Refactor
  • Split currency vs. cost-basis responsibilities into a clearer layered service/repository structure.
• Tests
  • Updated currency service tests to align with the new service construction and return flow.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d605bee5-9eb2-4698-9748-9b136649ee9a

📥 Commits

Reviewing files that changed from the base of the PR and between 965f005 and c2f124c.

📒 Files selected for processing (19)

• api/v3/handlers/currencies/create.go
• api/v3/handlers/currencies/create_cost_basis.go
• api/v3/handlers/currencies/get_cost_bases.go
• api/v3/handlers/currencies/handler.go
• api/v3/handlers/currencies/list.go
• api/v3/server/server.go
• app/common/currency.go
• cmd/server/wire.go
• cmd/server/wire_gen.go
• openmeter/currencies/adapter/adapter.go
• openmeter/currencies/adapter/currencies.go
• openmeter/currencies/costbasis.go
• openmeter/currencies/currency.go
• openmeter/currencies/models.go
• openmeter/currencies/repository.go
• openmeter/currencies/service.go
• openmeter/currencies/service/service.go
• openmeter/currencies/service/service_test.go
• openmeter/server/router/router.go

💤 Files with no reviewable changes (2)

• openmeter/currencies/models.go
• openmeter/currencies/adapter/currencies.go

✅ Files skipped from review due to trivial changes (2)

• openmeter/currencies/costbasis.go
• cmd/server/wire_gen.go

🚧 Files skipped from review as they are similar to previous changes (15)

• api/v3/handlers/currencies/create.go
• api/v3/handlers/currencies/create_cost_basis.go
• api/v3/handlers/currencies/get_cost_bases.go
• openmeter/currencies/currency.go
• openmeter/server/router/router.go
• cmd/server/wire.go
• api/v3/handlers/currencies/list.go
• api/v3/server/server.go
• openmeter/currencies/repository.go
• app/common/currency.go
• openmeter/currencies/service/service_test.go
• openmeter/currencies/service/service.go
• openmeter/currencies/adapter/adapter.go
• api/v3/handlers/currencies/handler.go
• openmeter/currencies/service.go

---

📝 Walkthrough

Walkthrough

The currencies package now uses split Repository and Service interfaces, with new domain types and validation inputs. The adapter, service implementation, wiring, and v3 handlers were updated to use the new contracts.

Changes

Currencies domain, service, and handler refactor

Layer / File(s)	Summary
Domain types openmeter/currencies/currency.go, openmeter/currencies/costbasis.go	CurrencyType, Currency, and CostBasis move into new files.
Repository and service contracts openmeter/currencies/repository.go, openmeter/currencies/service.go	Adapter becomes Repository, repository methods are split into currency and cost-basis interfaces, and Service gains separate currency/cost-basis interfaces plus new input types and validation.
Adapter and service implementation openmeter/currencies/adapter/adapter.go, openmeter/currencies/adapter/currencies.go, openmeter/currencies/service/service.go, openmeter/currencies/service/service_test.go	The adapter constructor now returns currencies.Repository; the service implementation is unexported and built from a repository; tests are updated to handle the new constructor result.
Dependency wiring app/common/currency.go, cmd/server/wire.go, cmd/server/wire_gen.go, api/v3/server/server.go, openmeter/server/router/router.go	Wiring now constructs a currency repository before the service, and public config/application fields use currencies.Service.
v3 handler updates api/v3/handlers/currencies/handler.go, create.go, create_cost_basis.go, list.go, get_cost_bases.go	The handler stores a service field, and currency/cost-basis handlers call h.service instead of h.currencyService.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• openmeterio/openmeter#3891: Touches the same v3 currencies handler area and the h.currencyService to h.service wiring pattern.
• openmeterio/openmeter#4585: Overlaps with the same currencies-package refactor and service/wiring updates.
• openmeterio/openmeter#4631: Also changes currency domain/model shape and symbol handling in the same area.

Suggested reviewers: mark-vass-konghq

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: a refactor of the currencies service package with no functional changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/currencies

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

openmeter/currencies/service.go (1)

40-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap these validators with models.NewNillableGenericValidationError.

ListCurrenciesInput, CreateCurrencyInput, CreateCostBasisInput, and ListCostBasesInput should return models.NewNillableGenericValidationError(errors.Join(errs...)) instead of a raw errors.Join(...). OrderBy.Validate() should use the same validation error type for the invalid case too.

🤖 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/currencies/service.go` around lines 40 - 46, The validation paths
are returning raw errors instead of the project’s nillable generic validation
type. Update the validation logic in ListCurrenciesInput, CreateCurrencyInput,
CreateCostBasisInput, and ListCostBasesInput so they wrap the joined validation
errors with models.NewNillableGenericValidationError, and make OrderBy.Validate
return that same error type for invalid values instead of a plain fmt.Errorf.
Use the existing validation helpers and the OrderBy.Validate method as the main
points to adjust.

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.

Inline comments:
In `@openmeter/currencies/currency.go`:
- Around line 12-19: CurrencyType.Validate currently fails fast by returning a
plain fmt.Errorf in the default branch, but it should follow the project
validation pattern. Update Validate() to build a var errs []error, append the
invalid-type error when t is not CurrencyTypeCustom or CurrencyTypeFiat, and
return models.NewNillableGenericValidationError(errors.Join(errs...)) instead of
returning immediately.

In `@openmeter/currencies/service/service_test.go`:
- Around line 71-73: `New` now returns an error, so the test helper and any
other `New(...)` call sites in this file must be updated to handle the second
return value. In `newTestService`, create the service and error from
`New(&fakeAdapter{custom: custom})`, then assert the error is nil with
`require.NoError(t, err)` before returning the service; apply the same pattern
to the other `New` usages in this test file.

---

Nitpick comments:
In `@openmeter/currencies/service.go`:
- Around line 40-46: The validation paths are returning raw errors instead of
the project’s nillable generic validation type. Update the validation logic in
ListCurrenciesInput, CreateCurrencyInput, CreateCostBasisInput, and
ListCostBasesInput so they wrap the joined validation errors with
models.NewNillableGenericValidationError, and make OrderBy.Validate return that
same error type for invalid values instead of a plain fmt.Errorf. Use the
existing validation helpers and the OrderBy.Validate method as the main points
to adjust.

🪄 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: 5bfddfd7-8889-4a81-8fe8-5975ce6bd14a

📥 Commits

Reviewing files that changed from the base of the PR and between 0eede87 and 73d9a3c.

📒 Files selected for processing (17)

• api/v3/handlers/currencies/create.go
• api/v3/handlers/currencies/create_cost_basis.go
• api/v3/handlers/currencies/get_cost_bases.go
• api/v3/handlers/currencies/handler.go
• api/v3/handlers/currencies/list.go
• app/common/currency.go
• cmd/server/wire.go
• cmd/server/wire_gen.go
• openmeter/currencies/adapter/adapter.go
• openmeter/currencies/adapter/currencies.go
• openmeter/currencies/costbasis.go
• openmeter/currencies/currency.go
• openmeter/currencies/models.go
• openmeter/currencies/repository.go
• openmeter/currencies/service.go
• openmeter/currencies/service/service.go
• openmeter/currencies/service/service_test.go

💤 Files with no reviewable changes (2)

• openmeter/currencies/adapter/currencies.go
• openmeter/currencies/models.go

[greptile-apps]

Greptile Summary

This PR refactors the currencies service package with no intended functional changes: it splits the monolithic models.go into focused files (currency.go, costbasis.go, service.go), renames Adapter → Repository with sub-interfaces, makes the service struct unexported, and changes New to return (currencies.Service, error).

• models.go is deleted and its content is redistributed — Validate() methods now call models.NewNillableGenericValidationError before returning, but the service layer still re-wraps the result with models.NewGenericValidationError, producing double-prefixed messages like \"validation error: invalid input parameters: validation error: …\".
• Wire DI is correctly regenerated: adapter and service construction are now separate steps (NewCurrencyAdapter + NewCurrencyService), matching the new two-return-value New signature.
• The CurrencyService sub-interface is retained but is now narrower than the top-level Service; all external call sites within the diff have been updated to use currencies.Service.

Confidence Score: 4/5

Safe to merge with one fix: validation error messages now carry a redundant "validation error:" prefix because Validate() wraps errors before the service layer wraps them again.

The refactor is mechanical and well-executed overall. The one present defect is that Validate() methods now return a GenericValidationError while the service layer unconditionally wraps the return value in another GenericValidationError, producing messages like "validation error: invalid input parameters: validation error: namespace is required". Type-based checks still work, but the error string visible to callers and logs changed in a way that contradicts the "no functional changes" goal.

openmeter/currencies/service.go (Validate() wrapping) and openmeter/currencies/service/service.go (service-layer re-wrapping) together produce the double-prefix error messages.

Important Files Changed

Filename	Overview
openmeter/currencies/service.go	Merged models.go content into service.go and introduced NewNillableGenericValidationError in Validate() methods; this causes double-wrapping when the service layer also wraps with NewGenericValidationError.
openmeter/currencies/service/service.go	Service struct made unexported, New now returns (currencies.Service, error) with a nil-repo guard; service methods unchanged except receiver rename. Double-wraps validation errors due to the change in Validate() methods.
openmeter/currencies/service/service_test.go	Updated all New() call sites to unpack the (Service, error) return and assert no error; correctly handled.
openmeter/currencies/repository.go	Renamed Adapter → Repository and split into CurrencyRepository + CostBasisRepository sub-interfaces; clean layering change.
app/common/currency.go	Split single NewCurrencyService into NewCurrencyAdapter + NewCurrencyService; cleaner DI wiring with proper error propagation.
openmeter/currencies/adapter/adapter.go	Return type changed from Adapter to Repository; added error wrapping for config validation; compile-time interface assertion updated.
cmd/server/wire_gen.go	Regenerated wiring to split adapter and service construction steps; straightforward mechanical update.
openmeter/currencies/currency.go	Extracted Currency and CurrencyType definitions from models.go into a dedicated file; no logic changes.
openmeter/currencies/costbasis.go	Extracted CostBasis struct from models.go into a dedicated file; no logic changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant H as HTTP Handler
    participant S as currencies.Service (interface)
    participant Svc as service.service (impl)
    participant R as currencies.Repository (interface)
    participant A as adapter.adapter (impl)
    participant DB as Ent DB

    H->>S: ListCurrencies / CreateCurrency / ListCostBases / CreateCostBasis
    S->>Svc: dispatch
    Svc->>Svc: params.Validate()
    alt validation fails
        Svc-->>H: GenericValidationError (double-wrapped)
    end
    Svc->>R: via transaction.Run
    R->>A: dispatch
    A->>DB: SQL query
    DB-->>A: rows
    A-->>R: typed result
    R-->>Svc: result
    Svc-->>H: result / error

Loading

%%{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"}}}%%
sequenceDiagram
    participant H as HTTP Handler
    participant S as currencies.Service (interface)
    participant Svc as service.service (impl)
    participant R as currencies.Repository (interface)
    participant A as adapter.adapter (impl)
    participant DB as Ent DB

    H->>S: ListCurrencies / CreateCurrency / ListCostBases / CreateCostBasis
    S->>Svc: dispatch
    Svc->>Svc: params.Validate()
    alt validation fails
        Svc-->>H: GenericValidationError (double-wrapped)
    end
    Svc->>R: via transaction.Run
    R->>A: dispatch
    A->>DB: SQL query
    DB-->>A: rows
    A-->>R: typed result
    R-->>Svc: result
    Svc-->>H: result / error

Loading

_{Reviews (3): Last reviewed commit: "fix: return validation error for invalid..." | Re-trigger Greptile}