Skip to content

Conversation

@turip
Copy link
Member

@turip turip commented Jul 28, 2025

Overview

If graduating tiered price's usage is set to nil we are not generating lines.

Existing invoices are unchanged, as the change does not change the usage based's qty and totals.

Summary by CodeRabbit

  • Bug Fixes

    • Zero quantity is now accepted as valid for flat fee line items, improving billing accuracy.
    • Billing logic now includes detailed lines for zero-quantity or zero-priced usage in graduated tiered pricing.
  • Tests

    • Updated test cases to expect and validate zero-quantity or zero-priced usage lines in tiered pricing scenarios.
    • Adjusted invoice tests to verify the presence and correctness of zero-usage or flat fee child lines.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 28, 2025

📝 Walkthrough

Walkthrough

Validation logic for flat fee quantities was relaxed to permit zero values. A new method for retrieving child lines by unique reference ID was introduced. The graduated tiered pricing logic was updated to always generate detailed lines for non-nil unit prices, even with zero quantity. Related tests were updated to reflect these changes.

Changes

Cohort / File(s) Change Summary
Flat Fee Validation & LineChildren Utility
openmeter/billing/invoiceline.go
Relaxed flat fee quantity validation to allow zero; updated error message; added GetByChildUniqueReferenceID method to LineChildren for retrieving child lines by unique reference ID.
Graduated Tiered Pricing Logic
openmeter/billing/service/lineservice/pricegraduatedtiered.go
Removed positive quantity check before creating detailed lines for tier unit prices; now creates lines whenever unit price is non-nil, regardless of quantity.
Graduated Tiered Pricing Tests
openmeter/billing/service/lineservice/pricegraduatedtiered_test.go
Updated test tiers to include explicit zero-valued unit prices; modified test cases to expect zero-quantity usage lines for tier 1; adjusted expected outputs accordingly.
Invoice Hierarchy Tests
test/billing/invoice_test.go
Adjusted test expectations to account for new inclusion of zero-quantity usage and flat fee lines as children; updated assertions to validate presence and totals of these lines in the invoice split hierarchy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15–20 minutes

Possibly related PRs

  • fix: only support qty=1 for flat fee lines #2893: Enforces root-level flat fee lines to have quantity exactly 1, modifying the same ValidateFee method as this PR, but with stricter validation logic. Both PRs adjust flat fee quantity validation, making them code-level related.

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4cc42 and 223d0ea.

📒 Files selected for processing (4)
  • openmeter/billing/invoiceline.go (2 hunks)
  • openmeter/billing/service/lineservice/pricegraduatedtiered.go (1 hunks)
  • openmeter/billing/service/lineservice/pricegraduatedtiered_test.go (4 hunks)
  • test/billing/invoice_test.go (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
openmeter/billing/service/lineservice/pricegraduatedtiered.go (1)

Learnt from: chrisgacsal
PR: #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).

test/billing/invoice_test.go (2)

Learnt from: chrisgacsal
PR: #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).

Learnt from: GAlexIHU
PR: #2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like openmeter/entitlement/metered/lateevents_test.go may use variables like meterSlug and namespace without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.

openmeter/billing/invoiceline.go (1)

Learnt from: chrisgacsal
PR: #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).

openmeter/billing/service/lineservice/pricegraduatedtiered_test.go (2)

Learnt from: chrisgacsal
PR: #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).

Learnt from: GAlexIHU
PR: #2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like openmeter/entitlement/metered/lateevents_test.go may use variables like meterSlug and namespace without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.

🧬 Code Graph Analysis (2)
test/billing/invoice_test.go (1)
openmeter/billing/totals.go (1)
  • Totals (9-26)
openmeter/billing/service/lineservice/pricegraduatedtiered_test.go (6)
api/api.gen.go (1)
  • UnitPrice (6917-6923)
api/client/go/client.gen.go (1)
  • UnitPrice (6333-6339)
openmeter/productcatalog/price.go (3)
  • UnitPrice (470-475)
  • PriceTierUnitPrice (803-806)
  • InArrearsPaymentTerm (21-21)
openmeter/ent/db/billinginvoiceline/where.go (4)
  • Amount (108-110)
  • Name (92-94)
  • Quantity (168-170)
  • ChildUniqueReferenceID (178-180)
openmeter/ent/db/billinginvoicelinediscount/where.go (3)
  • Amount (111-113)
  • Quantity (126-128)
  • ChildUniqueReferenceID (96-98)
openmeter/ent/db/billinginvoicelineusagediscount/where.go (2)
  • Quantity (111-113)
  • ChildUniqueReferenceID (96-98)
⏰ 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). (9)
  • GitHub Check: Quickstart
  • GitHub Check: Commit hooks
  • GitHub Check: Lint
  • GitHub Check: Build
  • GitHub Check: E2E
  • GitHub Check: CI
  • GitHub Check: Developer environment
  • GitHub Check: Test
  • GitHub Check: Analyze (go)
🔇 Additional comments (11)
openmeter/billing/service/lineservice/pricegraduatedtiered.go (1)

43-43: LGTM! Change correctly implements the PR objective.

The modification removes the positive quantity check (in.Quantity.IsPositive()) from the condition, ensuring that usage-based invoice lines are generated whenever a unit price exists, even with zero quantity. This aligns perfectly with the PR objective to generate usage-based lines when price is set to 0.

openmeter/billing/service/lineservice/pricegraduatedtiered_test.go (4)

25-27: Good addition of zero unit price to test data.

Adding a zero unit price to the first tier ensures the test data properly exercises the new behavior where zero-priced usage lines are generated.


74-82: Correctly validates zero-quantity usage line generation.

The test now expects a zero-quantity usage line with zero unit price, which properly validates that the system generates usage-based lines even when usage is nil, aligning with the PR objective.


97-103: Proper test coverage for zero-quantity tier 1 usage.

The addition of the zero-priced usage line for tier 1 with quantity 5 ensures comprehensive test coverage of the new behavior where usage lines are generated regardless of quantity when unit price is non-nil.


184-190: Appropriate test update for minimum spend scenario.

The test correctly expects a zero-quantity usage line before the minimum spend line, ensuring that the new behavior is properly validated in minimum spend scenarios.

test/billing/invoice_test.go (4)

401-402: LGTM! Test correctly validates the new child line behavior.

The test now properly expects 2 child lines instead of 1, reflecting the change where zero-quantity usage lines and flat fee lines are now included in the tiered graduated pricing hierarchy. The use of GetByChildUniqueReferenceID is the correct approach for retrieving specific child lines by their unique reference.


403-404: LGTM! Flat fee line validation is correctly implemented.

The test properly retrieves the flat fee line using the expected unique reference ID "graduated-tiered-1-flat-price" and validates its totals. The assertion that the flat fee line has an Amount and Total of 100 aligns with the tiered pricing structure where the first tier has a flat price of 100.

Also applies to: 409-409


438-440: LGTM! Test correctly expects zero-quantity usage lines.

The change from expecting 0 children to 1 child aligns with the PR objective. Even when there's no usage, the system now generates a zero-quantity usage line, which this test properly validates.


442-445: LGTM! Zero-quantity usage line validation is correct.

The test properly retrieves the usage-based line with unique reference ID "graduated-tiered-1-price-usage" and validates that its totals are zero. This correctly verifies the new behavior where zero-quantity usage lines are generated even when there's no actual usage, ensuring detailed invoice line representation.

openmeter/billing/invoiceline.go (2)

532-534: LGTM! Validation logic correctly updated to allow zero quantities.

The change from !IsPositive() to IsNegative() properly allows zero quantity flat fee lines while still preventing negative quantities. The updated error message accurately reflects the new validation rule. This aligns with the PR objective of generating usage-based invoice lines even when quantities are zero.


789-794: LGTM! Well-implemented method for retrieving child lines by reference ID.

The implementation correctly uses lo.FindOrElse for safe searching and lo.FromPtr for safe pointer dereferencing. The method follows the same pattern as the existing GetByID method and provides useful functionality for retrieving child lines by their unique reference ID.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/always-generate-usage-based-lines-for-zero-usage

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@turip turip force-pushed the fix/always-generate-usage-based-lines-for-zero-usage branch from 4cca5f8 to b7f4ae7 Compare July 28, 2025 14:33
If graduating tiered price's usage is set to nil we are not generating
lines.
@turip turip force-pushed the fix/always-generate-usage-based-lines-for-zero-usage branch from b7f4ae7 to 223d0ea Compare July 28, 2025 15:09
@turip turip added release-note/feature Release note: Exciting New Features area/billing labels Jul 28, 2025
@turip turip marked this pull request as ready for review July 28, 2025 15:10
@turip turip requested a review from a team as a code owner July 28, 2025 15:10
@turip turip merged commit 182a332 into main Jul 28, 2025
25 of 27 checks passed
@turip turip deleted the fix/always-generate-usage-based-lines-for-zero-usage branch July 28, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/billing release-note/feature Release note: Exciting New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants