feat: add filter and sort to billing profile#4377
Open
borosr wants to merge 3 commits into
Open
Conversation
borosr
added
the
release-note/ignore
borosr
force-pushed
the
feat/billing-v3-api-filters
branch
from
588e62e to
962d629
Compare
Contributor
|
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 Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds deep-object ChangesBilling profiles filtering and sorting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openmeter/billing/profile.go (1)
409-437: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winCollect namespace/expand errors into
errstoo forValidate()consistency.This method now aggregates some errors, but Line 410 and Line 414 still return early. To match project validation behavior, collect all field errors and return a single joined validation error.
As per coding guidelines,
openmeter/**/*.go: “ForValidate() errormethods, collect all validation issues intovar errs []errorand returnmodels.NewNillableGenericValidationError(errors.Join(errs...))instead of returning on the first invalid field”.🤖 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/billing/profile.go` around lines 409 - 437, ListProfilesInput.Validate currently returns early for Namespace and Expand errors instead of collecting them; update the method (ListProfilesInput.Validate) to append validation errors for Namespace and for i.Expand.Validate() into the errs slice (same as for ID/Name/OrderBy) and then return models.NewNillableGenericValidationError(errors.Join(errs...)); ensure you still validate Namespace (e.g., non-empty) and call i.Expand.Validate(), but push their resulting errors into errs rather than returning immediately.
🤖 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/src/billing/operations.tsp`:
- Around line 54-58: The example query in the `@query-decorated` filter
declaration is incorrect for the deep-object operator-style filter; update the
example to use operator form compatible with ListBillingProfilesParamsFilter
(e.g., use filter[name][eq]=my-profile or filter[name][contains]=my to match the
deepObject/explode=true shape) so clients send valid operator-style filter
parameters for the filter property.
In `@openmeter/billing/profile.go`:
- Around line 372-385: OrderBy.Values() currently returns only OrderByCreatedAt,
OrderByUpdatedAt, OrderByName, so OrderByID is wrongly excluded and
OrderBy.Validate() rejects "id"; update OrderBy.Values() to include OrderByID
(and any other missing OrderBy constants) so that OrderBy.Validate() sees "id"
as valid, and verify the same fix is applied to the other Values()/Validate()
pair referenced (lines around OrderByID) to ensure consistency.
In `@openmeter/billing/service/profile_test.go`:
- Around line 49-133: Add a new test case to the cases slice in profile_test.go
that exercises sorting by ID: create a test with name "SortByID" that sets input
billing.ListProfilesInput{Namespace: ns, OrderBy: "id", Order: sortx.OrderAsc
(or Desc)} and in assertAdapter assert that got.OrderBy == billing.OrderByID and
got.Order == the expected sortx order; this mirrors the existing
"SortByNameAsc"/"SortByUpdatedAt" cases and locks the new sort contract for
billing.ListProfilesInput and the mapping to billing.OrderByID.
---
Outside diff comments:
In `@openmeter/billing/profile.go`:
- Around line 409-437: ListProfilesInput.Validate currently returns early for
Namespace and Expand errors instead of collecting them; update the method
(ListProfilesInput.Validate) to append validation errors for Namespace and for
i.Expand.Validate() into the errs slice (same as for ID/Name/OrderBy) and then
return models.NewNillableGenericValidationError(errors.Join(errs...)); ensure
you still validate Namespace (e.g., non-empty) and call i.Expand.Validate(), but
push their resulting errors into errs rather than returning immediately.
🪄 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: ac63c23c-897a-444a-a18d-d5ad4b622855
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (7)
api/spec/packages/aip/src/billing/operations.tspapi/v3/api.gen.goapi/v3/handlers/billingprofiles/list.goopenmeter/billing/adapter/profile.goopenmeter/billing/httpdriver/profile.goopenmeter/billing/profile.goopenmeter/billing/service/profile_test.go
rolosp
previously approved these changes
May 18, 2026
borosr
force-pushed
the
feat/billing-v3-api-filters
branch
from
f28b4c6 to
b796418
Compare
borosr
force-pushed
the
feat/billing-v3-api-filters
branch
from
b796418 to
9ee3534
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openmeter/billing/profile.go (1)
410-417: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPlease aggregate all validation errors in
ListProfilesInput.Validate().Nice addition overall, but Line 411 and Line 415 still return early. That breaks the repo’s
Validate()aggregation rule and can hide additional invalid fields in one request.Suggested patch
func (i ListProfilesInput) Validate() error { - if i.Namespace == "" { - return errors.New("namespace is required") - } - - if err := i.Expand.Validate(); err != nil { - return fmt.Errorf("error validating expand: %w", err) - } - var errs []error + if i.Namespace == "" { + errs = append(errs, errors.New("namespace is required")) + } + + if err := i.Expand.Validate(); err != nil { + errs = append(errs, fmt.Errorf("error validating expand: %w", err)) + } + if i.ID != nil { if err := i.ID.Validate(); err != nil { errs = append(errs, err) } } @@ if i.OrderBy != "" { if err := i.OrderBy.Validate(); err != nil { errs = append(errs, err) } } return models.NewNillableGenericValidationError(errors.Join(errs...)) }As per coding guidelines,
openmeter/**/*.go: “ForValidate() errormethods, collect all validation issues intovar errs []errorand returnmodels.NewNillableGenericValidationError(errors.Join(errs...))instead of returning on the first invalid field”.Also applies to: 419-437
🤖 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/billing/profile.go` around lines 410 - 417, Change ListProfilesInput.Validate() to collect all field validation errors instead of returning early: create a slice var errs []error, append an error when i.Namespace == "" and append the wrapped error from i.Expand.Validate() if it fails (referencing i.Expand.Validate), then at the end if len(errs) > 0 return models.NewNillableGenericValidationError(errors.Join(errs...)) else return nil; apply the same aggregation pattern for the other checks in the function (lines handling pagination/filters referenced in the same Validate method).
🤖 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.
Outside diff comments:
In `@openmeter/billing/profile.go`:
- Around line 410-417: Change ListProfilesInput.Validate() to collect all field
validation errors instead of returning early: create a slice var errs []error,
append an error when i.Namespace == "" and append the wrapped error from
i.Expand.Validate() if it fails (referencing i.Expand.Validate), then at the end
if len(errs) > 0 return
models.NewNillableGenericValidationError(errors.Join(errs...)) else return nil;
apply the same aggregation pattern for the other checks in the function (lines
handling pagination/filters referenced in the same Validate method).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 141c3060-607d-45e8-bf77-4a05b9c8d852
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (7)
api/spec/packages/aip/src/billing/operations.tspapi/v3/api.gen.goapi/v3/handlers/billingprofiles/list.goopenmeter/billing/adapter/profile.goopenmeter/billing/httpdriver/profile.goopenmeter/billing/profile.goopenmeter/billing/service/profile_test.go
borosr
force-pushed
the
feat/billing-v3-api-filters
branch
from
9ee3534 to
448689c
Compare
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
2 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.
Add filters and sorting to the List App endpoint
What
Adds
filter[*]query parameters and asortquery parameter toGET /api/v3/openmeter/apps.Supported filters:
id,name.Supported sort fields:
id,name,createdAt (default),updatedAtwith optionalasc/descsuffix.Why
Callers need a way to narrow down billing profile results without client-side filtering, e.g. finding all billing profiles for a given type or sorted by creation time.
How
ListBillingProfilesParamsFilterand wiredsort/filterquery params intolistListBillingProfilesRequestTesting
Filter by billing profile name:
curl -s "http://localhost:8888/api/v3/openmeter/profiles?sort=createdAt%20asc&filter%5Bname%5D%5Bcontains%5D=sandbox"Sort by creation date descending:
curl -s "http://localhost:8888/api/v3/openmeter/profiles?sort=createdAt%20asc"Summary by CodeRabbit