feat(features): add meter id filter option to the list endpoint#4164
Open
feat(features): add meter id filter option to the list endpoint#4164
Conversation
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:
📝 WalkthroughWalkthroughAdds request-side meter_id filtering to Features listing: API spec and generated types gain a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Gateway
participant Handler as FeatureHandler
participant Parser as FilterParser
participant Connector as ProductCatalogConnector
participant DB as Database
Client->>API: GET /features?filter[meter_id][oeq]=<id>
API->>Handler: route + query params
Handler->>Parser: parse "filter[meter_id]" -> FilterString
Parser-->>Handler: FilterString / error
alt parse ok
Handler->>Connector: ListFeaturesParams{Namespace, Page, MeterIDs: FilterString}
Connector->>DB: apply filter.ApplyToQuery(..., MeterIDs, FieldMeterID)
DB-->>Connector: rows
Connector-->>Handler: features
Handler-->>API: 200 OK (paginated features)
else parse error
Handler-->>API: 400 Bad Request (InvalidParameters for filter[meter_id])
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🧹 Nitpick comments (3)
api/spec/packages/aip/src/features/operations.tsp (1)
16-17:@friendlyNamevs model name mismatch — intentional? 🤔The model is declared as
ListFeaturesParamsFilter(plural "Features") while@friendlyNameis"ListFeatureParamsFilter"(singular "Feature"). The parent params model isListFeaturesParams(plural), so the friendly name drifts from the established pattern. Given the generated Go type picks up the friendly name (ListFeatureParamsFilterper the summary), this will read a bit oddly alongsideListFeaturesParams. Might want to align them:✏️ Suggested fix
-@friendlyName("ListFeatureParamsFilter") +@friendlyName("ListFeaturesParamsFilter") model ListFeaturesParamsFilter {As per coding guidelines, "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/spec/packages/aip/src/features/operations.tsp` around lines 16 - 17, The `@friendlyName` value and the model identifier disagree: the model is declared as ListFeaturesParamsFilter (plural) but the decorator is `@friendlyName`("ListFeatureParamsFilter") (singular), causing mismatched generated Go types next to ListFeaturesParams; fix by making them consistent — either rename the model to ListFeatureParamsFilter to match the friendly name or change the decorator to `@friendlyName`("ListFeaturesParamsFilter") so it matches the model and the parent ListFeaturesParams; update the declaration where ListFeaturesParamsFilter and its `@friendlyName` are defined.openmeter/productcatalog/adapter/feature_test.go (1)
190-237: Nice coverage, but consider adding anEqcase too.Since
MeterIDsis now a fullFilterStringsupporting all StringFieldFilter ops (per the PR description), it'd be great to also exercise at leastEq(and maybeNe) so a future regression in predicate selection for meter_id doesn't slip through. Totally optional — theIn/empty/no-match cases already give solid confidence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openmeter/productcatalog/adapter/feature_test.go` around lines 190 - 237, Add an additional assertion in the "Should search by meter_id" test to exercise the Eq operator on MeterIDs: after creating the feature and the existing In-based checks, call connector.ListFeatures with feature.ListFeaturesParams{Namespace: namespace, MeterIDs: &filter.FilterString{Eq: lo.ToPtr(meterID)}} and assert it returns the single created feature (length 1 and Name "feature-1"); optionally add a negative Eq case (Eq set to "invalidid") asserting zero results. This uses the existing connector.ListFeatures, feature.ListFeaturesParams, and MeterIDs (filter.FilterString) symbols so the new checks slot into the same run func.api/v3/handlers/features/list.go (1)
57-70: Filter parsing + validation looks good.Parse →
Validate()→ assign is the right order, and theBadRequestErrorshape withfilter[meter_id]/InvalidParamSourceQuerymatches the pattern used elsewhere (e.g.api/v3/handlers/customers/list.go). Bonus: you're callingValidate()here, which the customers handler currently skips — nice hardening.One tiny optional thought: if more
filter[...]fields get added later, the repeatedBadRequestErrorblock will duplicate quickly. A small local helper likebadFilterParam(ctx, "filter[meter_id]", err)could keep things tidy, but totally fine to defer until the second filter shows up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v3/handlers/features/list.go` around lines 57 - 70, The code works but to reduce future duplication extract the repeated BadRequestError creation into a small helper (e.g., badFilterParam(ctx, field string, err error) apierrors.APIError) and replace the two inline apierrors.NewBadRequestError(...) calls in the params.Filter handling block (the block that calls filters.FromAPIFilterString(params.Filter.MeterId) and meterIDs.Validate()) with calls to that helper while still assigning req.MeterIDs on success; reference the existing symbols params.Filter, filters.FromAPIFilterString, meterIDs.Validate, req.MeterIDs and ListFeaturesRequest when implementing and using the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/spec/packages/aip/src/features/operations.tsp`:
- Line 40: The doc comment contains a typo: "filter[meter_id][oeq]=<id>" should
use the standard StringFieldFilter operator "eq"; update the comment to
"filter[meter_id][eq]=<id>" so generated OpenAPI docs and users follow the
correct filter syntax (search for the comment line referencing meter_id in
operations.tsp and replace "oeq" with "eq").
In `@api/v3/api.gen.go`:
- Around line 5055-5058: Update the spec example to use "eq" instead of "oeq"
for the meter_id filter in the source spec
(api/spec/packages/aip/src/features/operations.tsp) and then regenerate the
client code so the generated comment on the ListFeatureParamsFilter (the Filter
*ListFeatureParamsFilter comment in api/v3/api.gen.go) reflects
"filter[meter_id][eq]=<id>" consistently with other single-value filters;
specifically, change the example token from "oeq" to "eq" in the operations.tsp
comment and re-run the client generation step to propagate the fix into the
generated code.
In `@openmeter/productcatalog/adapter/feature_test.go`:
- Line 203: Update the inline URL-style comments in
openmeter/productcatalog/adapter/feature_test.go that currently show
"?filter[meter_id][oeq]=<meterID>" to correctly read
"?filter[meter_id][in]=<meterID>" (these comments document the In operator
exercised by the tests); search for occurrences of "[oeq]" in that file (notably
the three spots called out) and replace them with "[in]" so the comments match
the actual `In` operator being tested.
---
Nitpick comments:
In `@api/spec/packages/aip/src/features/operations.tsp`:
- Around line 16-17: The `@friendlyName` value and the model identifier disagree:
the model is declared as ListFeaturesParamsFilter (plural) but the decorator is
`@friendlyName`("ListFeatureParamsFilter") (singular), causing mismatched
generated Go types next to ListFeaturesParams; fix by making them consistent —
either rename the model to ListFeatureParamsFilter to match the friendly name or
change the decorator to `@friendlyName`("ListFeaturesParamsFilter") so it matches
the model and the parent ListFeaturesParams; update the declaration where
ListFeaturesParamsFilter and its `@friendlyName` are defined.
In `@api/v3/handlers/features/list.go`:
- Around line 57-70: The code works but to reduce future duplication extract the
repeated BadRequestError creation into a small helper (e.g., badFilterParam(ctx,
field string, err error) apierrors.APIError) and replace the two inline
apierrors.NewBadRequestError(...) calls in the params.Filter handling block (the
block that calls filters.FromAPIFilterString(params.Filter.MeterId) and
meterIDs.Validate()) with calls to that helper while still assigning
req.MeterIDs on success; reference the existing symbols params.Filter,
filters.FromAPIFilterString, meterIDs.Validate, req.MeterIDs and
ListFeaturesRequest when implementing and using the helper.
In `@openmeter/productcatalog/adapter/feature_test.go`:
- Around line 190-237: Add an additional assertion in the "Should search by
meter_id" test to exercise the Eq operator on MeterIDs: after creating the
feature and the existing In-based checks, call connector.ListFeatures with
feature.ListFeaturesParams{Namespace: namespace, MeterIDs:
&filter.FilterString{Eq: lo.ToPtr(meterID)}} and assert it returns the single
created feature (length 1 and Name "feature-1"); optionally add a negative Eq
case (Eq set to "invalidid") asserting zero results. This uses the existing
connector.ListFeatures, feature.ListFeaturesParams, and MeterIDs
(filter.FilterString) symbols so the new checks slot into the same run func.
🪄 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: 2ef7039c-72fd-4e35-91ac-a656c335d00a
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (6)
api/spec/packages/aip/src/features/operations.tspapi/v3/api.gen.goapi/v3/handlers/features/list.goopenmeter/productcatalog/adapter/feature.goopenmeter/productcatalog/adapter/feature_test.goopenmeter/productcatalog/feature/connector.go
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
api/v3/api.gen.go (1)
5060-5063:⚠️ Potential issue | 🟡 MinorPlease align the example operator for single
meter_idfiltering.On Line 5062,
filter[meter_id][oeq]=<id>is still shown; for a single ID example this should beeqto match the endpoint usage/docs.🔧 Suggested source-spec fix (then regenerate)
-// To filter features by meter_id add the following query param: filter[meter_id][oeq]=<id> +// To filter features by meter_id add the following query param: filter[meter_id][eq]=<id>As per coding guidelines
**/*.go: Never manually edit generated files marked with// Code generated by X, DO NOT EDIT.headers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v3/api.gen.go` around lines 5060 - 5063, Update the source OpenAPI/spec that generates the ListFeatureParamsFilter example so the single-meter_id operator uses "eq" instead of "oeq" (the generated api struct field is Filter *ListFeatureParamsFilter and the comment currently shows filter[meter_id][oeq]=<id>); do not edit the generated api.gen.go file directly—fix the spec (where the example or parameter operator is defined), regenerate the Go client so the comment and examples in ListFeatureParamsFilter are corrected to use filter[meter_id][eq]=<id>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@api/v3/api.gen.go`:
- Around line 5060-5063: Update the source OpenAPI/spec that generates the
ListFeatureParamsFilter example so the single-meter_id operator uses "eq"
instead of "oeq" (the generated api struct field is Filter
*ListFeatureParamsFilter and the comment currently shows
filter[meter_id][oeq]=<id>); do not edit the generated api.gen.go file
directly—fix the spec (where the example or parameter operator is defined),
regenerate the Go client so the comment and examples in ListFeatureParamsFilter
are corrected to use filter[meter_id][eq]=<id>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a4e428a-fbde-4a63-b2a1-5a319cd9e9f2
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (1)
api/v3/api.gen.go
tothandras
reviewed
Apr 17, 2026
tothandras
reviewed
Apr 17, 2026
…idate into the featureConnector.ListFeatures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openmeter/productcatalog/adapter/feature_test.go`:
- Around line 130-140: Remove the leftover debug print by deleting the
fmt.Println(c) call after the assert.Eventually block and also remove the
now-unused fmt import; the check is already performed by assert.Eventually (see
the assert.Eventually usage and the fmt.Println(c) statement) so simply delete
the print and the unused import to clean up the test output.
🪄 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: 8424922b-2ccc-416b-9953-3c04fc6ccbc7
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (4)
api/v3/api.gen.goapi/v3/handlers/features/list.goopenmeter/productcatalog/adapter/feature_test.goopenmeter/productcatalog/feature/connector.go
🚧 Files skipped from review as they are similar to previous changes (2)
- api/v3/handlers/features/list.go
- api/v3/api.gen.go
…ng group across 1 directory (#4151) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… in the oapi-codegen group across 1 directory (#4152) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…4171) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
….2 (#4140) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…idate into the featureConnector.ListFeatures
…/openmeter into feat/features-filter-meterid
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.
Overview
Extend the
GET /api/v3/openmeter/featuresendpoint with afilter[meter_id]query param. It implements all operations from theStringFieldFiltertypespec model.Notes for reviewer
Testing guide:
curl --request GET \ --url 'http://localhost:8888/api/v3/openmeter/features?filter%5Bmeter_id%5D%5Beq%5D=<meter id from the create meter response>'The response should look like this:
{ "data": [ { "created_at": "2026-04-17T10:32:46.450466Z", "description": "Hello feature", "id": "01KPDG04KJHJBT60PEG74JHSZN", "key": "meterid", "labels": {}, "meter": { "id": "01KPDB8KZXDW74V65CKYW5CKEN" }, "name": "Hello feature", "updated_at": "2026-04-17T10:32:46.450468Z" } ], "meta": { "page": { "size": 20, "number": 1, "total": 1 } } }Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor