chore: remove flat fee line constructor

[turip]

Overview

This patch makes sure that we are always using the usagebased wrapped fee lines in tests.

This is required as even thru the previous version is still working it is not something worth testing for: we should validate using the new format of the lines.

Notes for reviewer

The ubp wrapped flat fee lines is running reliably on prod for ages, so it's a no risk change.

Summary by CodeRabbit

• New Features
  • Added option to set a feature key when creating usage-based invoice lines.
• Refactor
  • Unified invoice line creation for flat-fee and usage-based charges under a single, consistent API.
  • Standardized invoice line classification to usage-based for improved consistency across billing flows.
• Tests
  • Updated test suites to align with the unified invoice line creation and revised line classification, ensuring coverage remains comprehensive.

[coderabbitai]

📝 Walkthrough

Walkthrough

Renames and repurposes the billing constructor: NewUsageBasedFlatFeeLine is removed and its usage replaced with NewFlatFeeLine. NewFlatFeeLine now builds a usage-based invoice line with optional featureKey. Tests and call sites are updated accordingly, including changes in expected line fields and type constants.

Changes

Cohort / File(s)	Summary of changes
Core billing API change openmeter/billing/invoiceline.go	Reworked NewFlatFeeLine to construct a usage-based line (type set to InvoiceLineTypeUsageBased), added variadic options (e.g., featureKey), and removed NewUsageBasedFlatFeeLine. Initialization now targets UsageBased instead of FlatFee.
Call-site update (non-test) openmeter/notification/internal/rule.go	Replaced billing.NewUsageBasedFlatFeeLine(...) with billing.NewFlatFeeLine(...) in newTestInvoicePayload.
Test updates — simple constructor rename openmeter/billing/httpdriver/invoice_test.go, test/app/stripe/invoice_test.go, test/billing/collection_test.go, test/billing/ubpflatfee_test.go	Switched constructor calls from NewUsageBasedFlatFeeLine to NewFlatFeeLine with the same NewFlatFeeLineInput. No other logic changes.
Test updates — structure/type expectations test/billing/invoice_test.go	Adjusted expectations from FlatFee to UsageBased fields and from InvoiceLineTypeFee to InvoiceLineTypeUsageBased; updated assertions (some require.Equal → ExpectJSONEqual).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• chore: fix test notification event #3121 — Modifies the same billing line constructors, aligning usage between NewFlatFeeLine and NewUsageBasedFlatFeeLine.
• fix: remove naked flat fee support #3170 — Converts constructor and handling paths for flat-fee to usage-based lines, overlapping with this PR’s API changes.
• chore: forbid API based flat fee line manipulation if ubp fs is active #2906 — Adds validations around creating/updating flat-fee lines under UBP, intersecting with this PR’s constructor/line-type changes.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title “chore: remove flat fee line constructor” accurately summarizes the core change of deleting the flat fee line constructor API and updating all call sites to the unified usage-based constructor. It is concise and specific, avoiding superfluous details while clearly conveying the intent. This phrasing aligns with the PR objectives and helps reviewers understand the primary modification at a glance.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/remove-flat-fee-line-constructor

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

openmeter/billing/invoiceline.go (1)

630-674: Update docstring to reflect UsageBased wrapper; consider a temporary shim for backward compatibility.

• The function now constructs a usage-based line backed by a flat price, but the comment still says “invoice-level flat fee line.” Please update to avoid confusion.
• Optional: add a deprecated alias for the removed NewUsageBasedFlatFeeLine to reduce breakage for external users for one release.

Example docstring change:

-// NewFlatFeeLine creates a new invoice-level flat fee line.
+// NewFlatFeeLine creates a usage-based invoice line backed by a flat price (UBP flat-fee wrapper).
+// Use WithFeatureKey(...) to associate a feature when needed.

Optional shim:

+// Deprecated: use NewFlatFeeLine. Will be removed in a future release.
+func NewUsageBasedFlatFeeLine(input NewFlatFeeLineInput, opts ...usageBasedLineOption) *Line {
+	return NewFlatFeeLine(input, opts...)
+}

test/billing/invoice_test.go (1)

48-48: Prefer pkg/clock for deterministic tests.

Use clock.Now() instead of time.Now() to keep tests stable and consistent with the suite’s time control.

-now := time.Now().Truncate(time.Second).In(time.UTC)
+now := clock.Now().Truncate(time.Second).In(time.UTC)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a43f61 and 7847a51.

📒 Files selected for processing (7)

• openmeter/billing/httpdriver/invoice_test.go (1 hunks)
• openmeter/billing/invoiceline.go (1 hunks)
• openmeter/notification/internal/rule.go (1 hunks)
• test/app/stripe/invoice_test.go (1 hunks)
• test/billing/collection_test.go (1 hunks)
• test/billing/invoice_test.go (10 hunks)
• test/billing/ubpflatfee_test.go (4 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-04-21T08:32:31.689Z

Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).

Applied to files:

• test/billing/ubpflatfee_test.go
• test/billing/invoice_test.go
• openmeter/billing/httpdriver/invoice_test.go

🧬 Code graph analysis (7)

test/billing/ubpflatfee_test.go (1)

openmeter/billing/invoiceline.go (2)

• NewFlatFeeLine (631-673)
• NewFlatFeeLineInput (592-616)

openmeter/notification/internal/rule.go (1)

openmeter/billing/invoiceline.go (2)

• NewFlatFeeLine (631-673)
• NewFlatFeeLineInput (592-616)

test/billing/collection_test.go (1)

openmeter/billing/invoiceline.go (2)

• NewFlatFeeLine (631-673)
• NewFlatFeeLineInput (592-616)

test/billing/invoice_test.go (4)

pkg/clock/clock.go (1)

• Now (14-21)

openmeter/billing/invoiceline.go (6)

• InvoiceLineTypeUsageBased (35-35)
• UsageBasedLine (764-777)
• Period (83-86)
• Line (311-325)
• NewFlatFeeLine (631-673)
• NewFlatFeeLineInput (592-616)

openmeter/productcatalog/price.go (4)

• Price (86-93)
• NewPriceFrom (369-391)
• FlatPrice (420-427)
• InAdvancePaymentTerm (20-20)

test/billing/suite.go (1)

• ExpectJSONEqual (571-581)

openmeter/billing/httpdriver/invoice_test.go (1)

openmeter/billing/invoiceline.go (1)

• NewFlatFeeLine (631-673)

openmeter/billing/invoiceline.go (1)

openmeter/billing/service/lineservice/service.go (1)

• Line (206-221)

test/app/stripe/invoice_test.go (1)

openmeter/billing/invoiceline.go (2)

• NewFlatFeeLine (631-673)
• NewFlatFeeLineInput (592-616)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: E2E
• GitHub Check: Quickstart
• GitHub Check: Lint
• GitHub Check: Test
• GitHub Check: Code Generators
• GitHub Check: Build
• GitHub Check: Analyze (go)

🔇 Additional comments (7)

test/app/stripe/invoice_test.go (1)

1304-1311: Constructor swap looks good.

Switch to billing.NewFlatFeeLine aligns with the unified constructor. No issues.

test/billing/collection_test.go (1)

244-251: Unified constructor usage is correct.

Both test cases now use billing.NewFlatFeeLine. Looks consistent with the new behavior.

Also applies to: 256-263

test/billing/ubpflatfee_test.go (1)

49-56: Migration to the new constructor is consistent.

All occurrences correctly use billing.NewFlatFeeLine; validations still check detailed fee lines and totals. LGTM.

Also applies to: 161-177, 241-256, 266-276

openmeter/billing/httpdriver/invoice_test.go (1)

59-69: LGTM on constructor change.

Switching to billing.NewFlatFeeLine matches the new creation path. No other adjustments needed.

openmeter/notification/internal/rule.go (1)

188-205: Test payload constructor update looks correct.

Using billing.NewFlatFeeLine here preserves the intended simulated invoice content.

test/billing/invoice_test.go (2)

252-252: Assertions updated for UsageBased wrapper are correct.

• Type switched to InvoiceLineTypeUsageBased.
• Expected price set via productcatalog.NewPriceFrom(FlatPrice).
• JSON equality helper aligns with structural changes.
• Lookups filter by usage-based type/feature key where appropriate.

Also applies to: 266-272, 285-285, 324-327, 363-371, 374-375

---

3979-3985: Constructor swaps in additional tests look good.

Updated to billing.NewFlatFeeLine across period persistence and deleted customer scenarios. No issues.

Also applies to: 4011-4017, 4090-4096, 4127-4133