feat(billing): add SearchOrgInvoices RQL endpoint#1536
Open
paanSinghCoder wants to merge 16 commits intomainfrom
Open
feat(billing): add SearchOrgInvoices RQL endpoint#1536paanSinghCoder wants to merge 16 commits intomainfrom
paanSinghCoder wants to merge 16 commits intomainfrom
Conversation
Introduce a new SearchOrgInvoices endpoint with RQL support while keeping ListInvoices behavior unchanged for existing consumers, and wire backend auth/service/repository plus regenerated frontier protobuf bindings. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 an organization-scoped invoice search feature across API handler, service, repository, models, mocks, tests, and authorization; also updates the Makefile PROTON_COMMIT used for proto generation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29242d54-19b4-4ba1-872f-e89c7f7065a6
⛔ Files ignored due to path filters (2)
proto/v1beta1/frontier.pb.gois excluded by!**/*.pb.go,!proto/**proto/v1beta1/frontierv1beta1connect/frontier.connect.gois excluded by!proto/**
📒 Files selected for processing (8)
Makefilebilling/invoice/invoice.gobilling/invoice/service.gointernal/api/v1beta1connect/billing_invoice.gointernal/api/v1beta1connect/interfaces.gointernal/api/v1beta1connect/mocks/invoice_service.gointernal/store/postgres/billing_invoice_repository.gopkg/server/connect_interceptors/authorization.go
Coverage Report for CI Build 24500005804Coverage decreased (-0.01%) to 41.805%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
AmanGIT07
reviewed
Apr 14, 2026
AmanGIT07
reviewed
Apr 14, 2026
Comment on lines
+316
to
+324
| if customerID == "" { | ||
| return nil, 0, errors.New("customer id is required") | ||
| } | ||
| if rqlQuery == nil { | ||
| return nil, 0, fmt.Errorf("%w: query is required", ErrBadInput) | ||
| } | ||
| if len(rqlQuery.GroupBy) > 0 { | ||
| return nil, 0, fmt.Errorf("%w: group_by is not supported", ErrBadInput) | ||
| } |
Contributor
There was a problem hiding this comment.
Validations can go in proto. (Caution: check if rqlQuery is validated there)
Contributor
Author
There was a problem hiding this comment.
customerID and GroupBy are not present on proto level thus makes sense to keep here.
Remove the query check.
AmanGIT07
reviewed
Apr 14, 2026
AmanGIT07
reviewed
Apr 14, 2026
paanSinghCoder
requested review from
AmanGIT07,
rohilsurana,
rsbh and
whoAbhishekSah
and removed request for
rohilsurana,
rsbh and
whoAbhishekSah
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/store/postgres/billing_invoice_repository.go (1)
314-333:⚠️ Potential issue | 🟠 MajorAdd a deterministic default order before paginating.
Line 330 paginates
withSorteven when the request does not specify any sort, so page boundaries are nondeterministic and clients can see duplicates/missing invoices across pages.Suggested fix
withSort, err := frontierutils.AddRQLSortInQuery(withSearch, rqlQuery) if err != nil { return nil, 0, fmt.Errorf("%w: %w", invoice.ErrBadInput, err) } + + if len(rqlQuery.Sort) == 0 { + withSort = withSort.OrderAppend( + goqu.I(TABLE_BILLING_INVOICES+".created_at").Desc(), + goqu.I(TABLE_BILLING_INVOICES+".id").Desc(), + ) + } countQuery, countParams, err := withSearch.Select(goqu.COUNT("*")).ToSQL()
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04d90ada-f210-4e7d-a6e5-293e01496ae8
📒 Files selected for processing (3)
billing/invoice/service.gointernal/api/v1beta1connect/billing_invoice.gointernal/store/postgres/billing_invoice_repository.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/api/v1beta1connect/billing_invoice.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
billing/invoice/service_search_org_invoices_test.go (1)
84-93: PreferErrorIsfor wrapped validation errors.For the nil-query and
group_bycases, asserting only message substrings is brittle. Addassert.ErrorIs(t, err, ErrBadInput)to validate the contract and keep the message checks as secondary.Proposed test hardening
_, _, err = svc.SearchOrgInvoices(context.Background(), "cust-1", true, nil) assert.Error(t, err) + assert.ErrorIs(t, err, ErrBadInput) assert.Contains(t, err.Error(), "query is required") @@ _, _, err = svc.SearchOrgInvoices(context.Background(), "cust-1", true, &rql.Query{ GroupBy: []string{"state"}, }) assert.Error(t, err) + assert.ErrorIs(t, err, ErrBadInput) assert.Contains(t, err.Error(), "group_by is not supported")internal/api/v1beta1connect/billing_invoice_test.go (1)
750-754: Avoid early return before mock expectation checks.Line 753 returns before
AssertExpectations, so error-path cases can skip mock verification.Suggested restructuring
got, err := handler.SearchOrgInvoices(context.Background(), tt.request) if tt.wantCode != 0 { assert.Error(t, err) assert.Equal(t, tt.wantCode, connect.CodeOf(err)) - return - } - - assert.NoError(t, err) - if tt.assertResp != nil { - tt.assertResp(t, got) + assert.Nil(t, got) + } else { + assert.NoError(t, err) + if tt.assertResp != nil { + tt.assertResp(t, got) + } } mockInvoiceService.AssertExpectations(t) mockCustomerService.AssertExpectations(t)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b292a96f-57e4-4776-9c10-9c5151698a4e
📒 Files selected for processing (2)
billing/invoice/service_search_org_invoices_test.gointernal/api/v1beta1connect/billing_invoice_test.go
AmanGIT07
reviewed
Apr 14, 2026
AmanGIT07
reviewed
Apr 15, 2026
AmanGIT07
reviewed
Apr 15, 2026
Contributor
|
@paanSinghCoder Please add some sample request and response |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/api/v1beta1connect/billing_invoice.go (1)
104-135:⚠️ Potential issue | 🟠 MajorValidate and normalize RQL before
GetByOrgIDearly-return paths.Right now, invalid RQL can return
200 OKforcustomer.ErrNotFound, and default pagination is skipped in that branch because validation/defaulting happens later.🛠️ Proposed fix
func (h *ConnectHandler) SearchOrgInvoices(ctx context.Context, request *connect.Request[frontierv1beta1.SearchOrgInvoicesRequest]) (*connect.Response[frontierv1beta1.SearchOrgInvoicesResponse], error) { errorLogger := NewErrorLogger() queryMsg := request.Msg.GetQuery() if queryMsg == nil { queryMsg = &frontierv1beta1.RQLRequest{} } + rqlQuery, err := utils.TransformProtoToRQL(queryMsg, invoice.SearchOrgInvoice{}) + if err != nil { + return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("failed to read rql query: %v", err)) + } + if err = rql.ValidateQuery(rqlQuery, invoice.SearchOrgInvoice{}); err != nil { + return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("failed to validate rql query: %v", err)) + } + if rqlQuery.Limit <= 0 { + rqlQuery.Limit = utils.DefaultLimit + } + if rqlQuery.Offset < 0 { + rqlQuery.Offset = utils.DefaultOffset + } + cust, err := h.customerService.GetByOrgID(ctx, request.Msg.GetOrgId()) if err != nil { if errors.Is(err, customer.ErrNotFound) { return connect.NewResponse(&frontierv1beta1.SearchOrgInvoicesResponse{ Invoices: []*frontierv1beta1.Invoice{}, Pagination: &frontierv1beta1.RQLQueryPaginationResponse{ - Offset: queryMsg.GetOffset(), - Limit: queryMsg.GetLimit(), + Offset: uint32(rqlQuery.Offset), + Limit: uint32(rqlQuery.Limit), TotalCount: 0, }, }), nil } @@ - rqlQuery, err := utils.TransformProtoToRQL(queryMsg, invoice.SearchOrgInvoice{}) - if err != nil { - return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("failed to read rql query: %v", err)) - } - if err = rql.ValidateQuery(rqlQuery, invoice.SearchOrgInvoice{}); err != nil { - return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("failed to validate rql query: %v", err)) - }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1302b844-eedb-4f66-b9e1-83a8b3ad8be5
📒 Files selected for processing (9)
billing/invoice/invoice.gobilling/invoice/service.gobilling/invoice/service_search_org_invoices_test.gointernal/api/v1beta1connect/billing_invoice.gointernal/api/v1beta1connect/billing_invoice_test.gointernal/api/v1beta1connect/interfaces.gointernal/api/v1beta1connect/mocks/invoice_service.gointernal/store/postgres/billing_invoice_repository.gopkg/server/connect_interceptors/authorization.go
✅ Files skipped from review due to trivial changes (2)
- billing/invoice/service_search_org_invoices_test.go
- billing/invoice/invoice.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/server/connect_interceptors/authorization.go
- internal/store/postgres/billing_invoice_repository.go
- internal/api/v1beta1connect/billing_invoice_test.go
- internal/api/v1beta1connect/interfaces.go
- billing/invoice/service.go
AmanGIT07
reviewed
Apr 16, 2026
Contributor
Author
|
Rename |
AmanGIT07
reviewed
Apr 16, 2026
…sistent pagination handling
…id org ID formats and service errors
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
3 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.
Summary
SearchOrgInvoicesRPC in FrontierService that supportsRQLRequestand pagination responseSearchOrgInvoices RQL Support Matrix
org_idnonzero_amount_onlytrueappliesamount > 0filter.query.limit50when missing/invalid.query.offset0when missing/invalid.query.searchquery.sortquery.filtersquery.group_bygroup_by is not supported).Filterable / Sortable Fields
idprovider_idcustomer_idstatecurrencyamounthosted_urldue_ateffective_atcreated_atperiod_start_atperiod_end_atSearchable Fields
idprovider_idstatecurrencyhosted_urlOperator Support (practical)
stringeq,neq,like,notlike,in,notin,empty,notemptynumbereq,neq,gt,gte,lt,ltedatetimeeq,neq,gt,gte,lt,lteTest plan
make protoand verify no generated-file diffs remainListInvoicesconsumers still behave unchangedSearchOrgInvoiceswith filters/sort/search/limit/offset