chore: rename invoice types #3802
Merged
Conversation
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
Rename standard invoice related types from Invoice to StandardInvoice and adjust other calls. This is a prerequisite to split gathering invoice handling from standard invoices.
turip
added
the
release-note/ignore
Contributor
📝 WalkthroughWalkthroughThis PR systematically renames the billing domain's invoice and line types from generic Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openmeter/billing/stdinvoice_test.go (1)
14-56: StandardLines elements need to be pointers, but DetailedLines are correct as-is.
StandardLinesis[]*StandardLine(pointer slice), so each element must be&StandardLine{...}. However,DetailedLinesis[]DetailedLine(non-pointer slice), so the current value literals are correct—no&needed there.Suggested fix
-lines := StandardLines{ - { +lines := StandardLines{ + &StandardLine{ StandardLineBase: StandardLineBase{ ManagedResource: models.NewManagedResource(models.ManagedResourceInput{ Name: "usage-based-line", Description: lo.ToPtr("index=1"), }), Period: Period{ Start: time.Now().Add(time.Hour * 24), }, }, DetailedLines: DetailedLines{ { DetailedLineBase: DetailedLineBase{ ManagedResource: models.NewManagedResource(models.ManagedResourceInput{ Name: "child-2", Description: lo.ToPtr("index=1.1"), }), Index: lo.ToPtr(1), }, }, { DetailedLineBase: DetailedLineBase{ ManagedResource: models.NewManagedResource(models.ManagedResourceInput{ Name: "child-1", Description: lo.ToPtr("index=1.0"), }), Index: lo.ToPtr(0), }, }, }, }, - { + &StandardLine{ StandardLineBase: StandardLineBase{ ManagedResource: models.NewManagedResource(models.ManagedResourceInput{ Name: "usage-based-line", Description: lo.ToPtr("index=0"), }), Period: Period{ Start: time.Now(), }, }, }, }
🤖 Fix all issues with AI agents
In `@openmeter/billing/customer.go`:
- Around line 13-16: The comment above the type assertion is incorrect: it
references streaming.CustomerUsageAttribution while the code asserts var _
streaming.Customer = &InvoiceCustomer{}; update the comment to accurately
describe that InvoiceCustomer implements the streaming.Customer interface (or
change the assertion if you intended CustomerUsageAttribution). Locate the
InvoiceCustomer declaration and replace the misleading comment so it matches the
asserted interface name (streaming.Customer) and clearly states purpose, e.g.,
"InvoiceCustomer implements the streaming.Customer interface used to query
customer usage."
- Around line 38-44: InvoiceCustomer's CustomerID is required but currently has
`json:"customerId,omitempty"` which can drop it during marshaling; remove
`omitempty` from the `CustomerID` struct tag in the InvoiceCustomer type so the
field is always emitted (update the tag on the CustomerID field in the
InvoiceCustomer struct), and run/check any serialization tests and callers of
InvoiceCustomer.Validate() to ensure they no longer rely on the field being
omitted.
- Around line 57-71: The Validate method on InvoiceCustomer lacks a nil receiver
guard and will panic if called on a nil pointer; update InvoiceCustomer.Validate
to first check if i == nil and return a clear error (e.g., "invoice customer is
nil") before accessing i.CustomerID, i.Key, or i.Name so all subsequent
validations are safe.
In `@openmeter/billing/gatheringinvoice.go`:
- Around line 18-52: The Validate method CreatePendingInvoiceLinesInput.Validate
currently mutates caller data and can panic if a nil pointer exists in c.Lines;
change the loop to guard against nil entries (if line == nil append a validation
error and continue) and operate on a local copy before setting Currency (e.g.,
copy the pointed-to Line into a local variable like l := *line) so you set
l.Currency and call l.Validate() and inspect l.InvoiceID, l.DetailedLines,
l.ParentLineID, and l.SplitLineGroupID — this prevents altering the original
slice elements and avoids nil dereference panics.
In `@openmeter/billing/stdinvoiceline.go`:
- Around line 251-323: ValidateUsageBased currently dereferences i.UsageBased
and i.UsageBased.Price and can panic if those pointers are nil; update
ValidateUsageBased (and the similar UsageBasedLine.Validate at the other
location) to defensively check for nil before calling methods or accessing
fields: if i.UsageBased == nil append a validation error like "usageBased:
missing" and skip calling i.UsageBased.Validate or accessing i.UsageBased.Price,
and likewise guard the call to RateCardDiscounts.ValidateForPrice by only
invoking it when i.UsageBased != nil and i.UsageBased.Price != nil; ensure you
append readable fmt.Errorf-wrapped errors to errs and return
errors.Join(errs...) as before.
🧹 Nitpick comments (1)
openmeter/billing/customer.go (1)
18-30: Avoid calling GetUsageAttribution twice.
Tiny, but caching it reads cleaner and avoids repeated work.♻️ Proposed refactor
func NewInvoiceCustomer(cust customer.Customer) InvoiceCustomer { ic := InvoiceCustomer{ Key: cust.Key, CustomerID: cust.ID, Name: cust.Name, BillingAddress: cust.BillingAddress, } // If the customer has a usage attribution, we add it to the invoice customer // We use the validator but this is not an error, we allow non usage based invoices without usage attribution. - if err := cust.GetUsageAttribution().Validate(); err == nil { - ic.UsageAttribution = lo.ToPtr(cust.GetUsageAttribution()) + ua := cust.GetUsageAttribution() + if err := ua.Validate(); err == nil { + ic.UsageAttribution = lo.ToPtr(ua) } return ic }
tothandras
approved these changes
Jan 22, 2026
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
Rename standard invoice related types from Invoice to StandardInvoice and adjust other calls.
This is a prerequisite to split gathering invoice handling from standard invoices.
This is a non-breaking change as it is only a type/file rename.
Notes for reviewer
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.