Skip to content

Conversation

@turip
Copy link
Member

@turip turip commented Jul 31, 2025

Overview

This patch removes the "naked" flat fee support, as the API and everything else have been already changed to only support usage-based wrapped flat fees.

Readding the migration to make sure that any remaining lines are properly converted. This fixes the regression that unless the .billing.featureSwitches.useUsageBasedFlatFeeLines=true setting was set, opensource invoicing was not functioning properly if a subscription was started.

Summary by CodeRabbit

  • Bug Fixes

    • Flat fee invoice lines are now consistently restricted and validated, regardless of previous feature flag settings.
  • Refactor

    • All billing logic now uses usage-based pricing for flat fee items, simplifying invoice line handling.
    • Feature flag and related configuration for flat fee line types have been removed.
    • Internal functions and tests updated to reflect the exclusive use of usage-based lines.
  • Chores

    • Database migration script added to convert existing flat fee invoice lines to usage-based pricing lines.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

📝 Walkthrough

Walkthrough

This change removes the UseUsageBasedFlatFeeLines feature flag and all related conditional logic throughout the billing codebase. It updates handler initialization, validation, helper functions, synchronization logic, and test cases to exclusively support usage-based flat fee invoice lines. A database migration script is added to convert existing flat fee lines to usage-based equivalents.

Changes

Cohort / File(s) Change Summary
Feature Flag & Handler Removal
app/common/billing.go, cmd/billing-worker/wire_gen.go, cmd/jobs/internal/wire_gen.go
Removed the UseUsageBasedFlatFeeLines feature flag from handler signatures and initialization. Updated handler construction to omit feature flag configuration.
Configuration Update
app/config/billing.go
Removed the UseUsageBasedFlatFeeLines field from the BillingFeatureSwitchesConfiguration struct.
Invoice Line Validation
openmeter/billing/httpdriver/invoiceline.go
Removed conditional validation logic based on the feature flag; flat fee lines are now always forbidden unconditionally.
Subscription Worker Logic
openmeter/billing/worker/subscription/feehelper.go, openmeter/billing/worker/subscription/sync.go
Removed all logic and helper code handling legacy flat fee invoice lines and feature flag checks. Now only usage-based flat fee lines are supported.
Test Adjustments
openmeter/billing/worker/subscription/sync_test.go
Updated tests to use and validate usage-based flat fee lines, adjusting assertions and helper logic accordingly.
Database Migration
tools/migrate/migrations/20250731141420_billing-migrate-flat-fee-lines.up.sql
Added a migration script to convert all existing flat fee invoice lines to usage-based flat fee lines, preserving historical data as detailed child lines.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

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 3a4024c and 3727d4a.

⛔ Files ignored due to path filters (1)
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • app/common/billing.go (2 hunks)
  • app/config/billing.go (1 hunks)
  • cmd/billing-worker/wire_gen.go (1 hunks)
  • cmd/jobs/internal/wire_gen.go (1 hunks)
  • openmeter/billing/httpdriver/invoiceline.go (3 hunks)
  • openmeter/billing/worker/subscription/feehelper.go (1 hunks)
  • openmeter/billing/worker/subscription/sync.go (1 hunks)
  • openmeter/billing/worker/subscription/sync_test.go (41 hunks)
  • tools/migrate/migrations/20250731141420_billing-migrate-flat-fee-lines.up.sql (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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`).
📚 Learning: in `productcatalog.usagebasedratecard`, the `billingcadence` field is a non-pointer `isodate.period`...
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:

  • openmeter/billing/worker/subscription/sync.go
  • openmeter/billing/worker/subscription/sync_test.go
  • app/config/billing.go
  • openmeter/billing/worker/subscription/feehelper.go
  • openmeter/billing/httpdriver/invoiceline.go
  • app/common/billing.go
📚 Learning: in the openmeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may us...
Learnt from: GAlexIHU
PR: openmeterio/openmeter#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.

Applied to files:

  • openmeter/billing/worker/subscription/sync.go
  • openmeter/billing/worker/subscription/sync_test.go
🧬 Code Graph Analysis (2)
openmeter/billing/worker/subscription/feehelper.go (2)
openmeter/billing/invoiceline.go (2)
  • InvoiceLineTypeUsageBased (33-33)
  • Line (304-318)
openmeter/productcatalog/price.go (1)
  • NewPriceFrom (369-391)
openmeter/billing/httpdriver/invoiceline.go (3)
openmeter/billing/invoiceline.go (1)
  • InvoiceLineTypeFee (31-31)
openmeter/ent/db/ent.go (1)
  • ValidationError (259-262)
openmeter/billing/errors.go (1)
  • ValidationError (119-119)
🔇 Additional comments (28)
app/config/billing.go (1)

37-37: LGTM! Clean removal of feature flag from configuration.

The removal of UseUsageBasedFlatFeeLines from the configuration struct is consistent with the PR objective to eliminate support for naked flat fees.

cmd/jobs/internal/wire_gen.go (1)

349-349: LGTM! Wire-generated code correctly reflects function signature change.

The removal of the billingFeatureSwitchesConfiguration parameter aligns with the updated NewBillingSubscriptionHandler signature after feature flag removal.

cmd/billing-worker/wire_gen.go (1)

291-291: LGTM! Consistent wire-generated code update.

The parameter removal matches the pattern in the jobs wire file and correctly reflects the updated handler signature.

openmeter/billing/httpdriver/invoiceline.go (3)

73-79: Correct enforcement of flat fee line restriction.

The unconditional validation prevents creation of flat fee lines, enforcing the transition to usage-based lines. This is a breaking change but aligns with the PR objective.


714-718: Consistent validation for new lines in merge operation.

The validation correctly prevents creation of new flat fee lines during invoice line merging, maintaining consistency with the create operation.


740-744: Proper validation for updated lines in merge operation.

The validation ensures that updates to existing lines cannot result in flat fee lines, completing the enforcement across all line operations.

openmeter/billing/worker/subscription/sync.go (2)

31-34: Clean removal of feature flag from worker configuration.

The UseUsageBasedFlatFeeLines field removal from FeatureFlags struct is consistent with the overall feature flag elimination.


618-625: Correct implementation of usage-based line creation for flat prices.

The logic properly constructs usage-based lines with embedded flat price structures, maintaining the pricing semantics while transitioning away from naked flat fee lines. The FlatPrice construction with perUnitAmount and PaymentTerm preserves the original pricing intent.

openmeter/billing/worker/subscription/feehelper.go (3)

15-28: LGTM! Flat fee detection logic updated correctly.

The function now correctly identifies flat fees as usage-based lines with flat price type, aligning with the removal of legacy flat fee line types.


30-49: LGTM! Function correctly simplified for usage-based model.

The function now exclusively handles usage-based lines with proper error handling and validation. The logic correctly extracts the flat price amount from the embedded price configuration.


51-72: LGTM! Function correctly updated for usage-based pricing model.

The function properly validates usage-based lines, extracts and updates the flat price amount, and correctly recreates the price object using productcatalog.NewPriceFrom(). The error handling is appropriate for the simplified model.

app/common/billing.go (2)

74-74: LGTM! Function call updated to match new signature.

The call to NewBillingSubscriptionHandler correctly removes the fsConfig parameter, aligning with the removal of feature flag configuration.


122-130: LGTM! Handler initialization simplified correctly.

The function signature and implementation have been appropriately updated to remove feature flag dependencies, simplifying the billing subscription handler creation process.

tools/migrate/migrations/20250731141420_billing-migrate-flat-fee-lines.up.sql (4)

10-68: LGTM! Standard ULID generation implementation.

The ULID generation function correctly implements the standard specification with proper timestamp encoding, entropy generation, and Crockford Base32 encoding. Using pg_temp ensures automatic cleanup.


78-87: LGTM! Migration function properly defined.

The function signature, parameter, and variable declarations are appropriate for migrating individual flat fee lines to usage-based pricing lines.


225-225: LGTM! Migration execution correctly targets relevant data.

The execution statement properly targets only flat fee lines with valid status, ensuring complete migration without affecting already processed or invalid lines.


88-220: Migration JSON price format is correct

Verified that the migration’s JSON payload

{"type":"flat","amount":"","paymentTerm":""}

matches the productcatalog FlatPrice struct (json:"type", json:"amount", json:"paymentTerm" with FlatPriceType = "flat"). No further changes required.

openmeter/billing/worker/subscription/sync_test.go (11)

678-681: LGTM! Proper conversion from flat fee to usage-based price access.

The conversion correctly extracts the flat price from the usage-based line using AsFlat() and accesses the amount and payment term properties. Error handling is appropriately included.


732-735: LGTM! Consistent usage-based price conversion pattern.

The code follows the same correct pattern as the previous conversion, properly extracting flat price details from the usage-based line structure.


808-811: LGTM! Test expectation correctly updated for usage-based pricing.

The test expectation has been properly updated to use productcatalog.NewPriceFrom(productcatalog.FlatPrice{...}) which aligns with the new usage-based flat fee representation.


921-924: LGTM! Consistent test expectation pattern maintained.

The price expectation follows the established pattern for usage-based flat fee lines.


1283-1286: LGTM! Price expectation correctly structured.

The test properly expects a usage-based price structure with embedded flat price details.


3227-3231: LGTM! Manual edit logic properly updated for usage-based prices.

The manual edit code correctly:

  1. Extracts the flat price using AsFlat() with proper error handling
  2. Modifies the payment term on the extracted price
  3. Creates a new price structure and assigns it back to the usage-based line

This maintains the same functionality while working with the new usage-based price structure.


3565-3579: LGTM! Proper handling of config ID clearing for child lines.

The code correctly clears the FlatFee.ConfigID on child lines for both the expected invoice and the actual invoice after sync. This prevents mismatches due to config ID differences during test comparisons and aligns with the migration to usage-based lines.


3609-3611: LGTM! Price amount assertion correctly updated.

The test assertion properly extracts the flat price using AsFlat() and compares the amount, maintaining the same test logic with the new price structure.


4169-4172: LGTM! Discount test expectations properly updated.

The price expectations in the discount synchronization test correctly use the usage-based price structure with embedded flat prices, ensuring the discount functionality works properly with the new line type.

Also applies to: 4190-4193


4206-4211: LGTM! Discount amount assertion correctly updated for child line structure.

The discount amount assertion has been properly updated to:

  1. Access child lines from the usage-based line structure
  2. Verify the discount amount on the child line rather than directly on the flat fee

This correctly reflects the new structure where discounts are applied to child lines in the usage-based flat fee representation.


4622-4634: LGTM! Quantity and price assertions properly updated for usage-based lines.

The test helper method correctly:

  1. Checks for usage-based line type instead of flat fee
  2. Accesses quantity from line.UsageBased.Quantity
  3. Compares prices using the usage-based price structure
  4. Maintains proper error messages for debugging

This ensures test validation works correctly with the new usage-based flat fee line representation.

✨ 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/remove-naked-flat-fee-support

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 added release-note/deprecation Release note: Deprecations area/billing labels Jul 31, 2025
This patch removes the "naked" flat fee support, as the API and everything
else have been already changed to only support usage-based wrapped flat fees.

Readding the migration to make sure that any remaining lines are properly converted.
@turip turip force-pushed the fix/remove-naked-flat-fee-support branch from de368f5 to 3727d4a Compare July 31, 2025 15:37
@turip turip marked this pull request as ready for review July 31, 2025 15:38
@turip turip requested a review from a team as a code owner July 31, 2025 15:38
@turip turip added the release-note/bug-fix Release note: Bug Fixes label Jul 31, 2025
@turip turip merged commit 5904d91 into main Jul 31, 2025
28 checks passed
@turip turip deleted the fix/remove-naked-flat-fee-support branch July 31, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/billing release-note/bug-fix Release note: Bug Fixes release-note/deprecation Release note: Deprecations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants