feat: openmeter customer override api

[turip]

Overview

Implement the customer override API. This allows us in the current form to:

• Pin customers to different billing profiles (only)
• Create and delete customer overrides

Summary by CodeRabbit

• New Features

  • Enhanced filtering options for customer billing overrides, enabling filtering by various customer attributes.
  • Introduced a method to check for the existence of a customer based on their ID.
  • Consolidated customer override operations with an upsert method for more detailed billing profile responses.

• Refactor

  • Streamlined API endpoints by removing deprecated pagination parameters and unifying response structures.
  • Improved transaction management and error handling across billing and customer services.

• Tests

  • Added new test cases to validate customer name filtering and input validations.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request implements extensive modifications across billing and customer modules. Key changes include the addition of new fields and types for customer overrides, updates to API filtering parameters, and consolidation of service methods into a unified upsert operation. Invoice processing and worker methods are enhanced to accommodate detailed customer override information. Pagination parameters are removed in favor of streamlined queries, and customer existence checks and input validations are introduced. Minor adjustments in import ordering and test routines are also included.

Changes

File(s)	Change Summary
api/client/.../schemas.ts, api/openapi.cloud.yaml, api/openapi.yaml, api/spec/.../customeroverride.tsp	Added new fields and types for billing profile customer overrides (e.g., billingProfileWithOverrides, customer, BillingProfileCustomerOverrideExpand, BillingProfileCustomerOverrideWithDetails), expanded filtering parameters (e.g., customerId, customerName, etc.), and renamed parameters (e.g., includeDefaultProfile → includeAllCustomers).
openmeter/billing/adapter.go, openmeter/billing/adapter/customeroverride.go, openmeter/billing/httpdriver/customeroverride.go, openmeter/billing/httpdriver/handler.go, openmeter/billing/service.go	Updated adapter and service methods to consolidate override operations. Replaced separate create/update/get with a unified UpsertCustomerOverride method and adjusted query modifiers, with minor import and in-place modification changes.
openmeter/billing/invoice.go, openmeter/billing/service/invoice.go, openmeter/billing/service/invoiceline.go	Modified invoice processing by adding a CustomerID() method, expanding override data retrieval (e.g., adding Apps: true), and updating invoice line models to use CustomerOverrideWithDetails instead of pointer types.
openmeter/billing/validators/subscription/validator.go, openmeter/billing/worker/collect/collect.go, openmeter/billing/worker/subscription/sync.go	Replaced calls to GetProfileWithCustomerOverride with GetCustomerOverride and updated input structures, ensuring a consistent and enhanced retrieval of customer override data during worker processes and billing setup validations.
openmeter/customer/adapter/customer.go, openmeter/customer/httpdriver/apimapping.go, openmeter/customer/httpdriver/customer.go, openmeter/customer/adapter.go, openmeter/customer/service.go, openmeter/customer/service/customer.go, openmeter/server/server_test.go, openmeter/customer/customer.go	Added CustomerExists methods across customer adapters, services, and tests; introduced a Validate method for ListCustomersInput; and updated function visibility (e.g., renaming customerToAPI to CustomerToAPI) to improve customer data validation and consistency.
openmeter/server/router/billing.go	Introduced conditional billing-enabled checks and updated method signatures (adding extra parameters) for handling billing profile customer override endpoints.
api/spec/src/billing/invoices.tsp, api/spec/src/billing/profile.tsp, openmeter/billing/profile.go, openmeter/app/stripe/adapter/stripe.go, openmeter/productcatalog/subscription/service/create.go	Removed pagination limit/offset parameters and deleted the ProfileWithCustomerDetails type, reflecting a shift toward updated customer override handling and simplified pagination in API endpoints.
test/billing/customeroverride_test.go, test/billing/tax_test.go	Added new test cases for filtering customer overrides and updated tax configuration tests to align with new override input types and variable naming conventions.
pkg/lazy/lazy.go	Introduced a new generic function OnceValueErr for lazy initialization with error handling using sync.Once.

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 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.
  • Generate unit testing code for this file.
  • 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 generate unit testing code for this file.
  • @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 generate unit testing code.
  • @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.

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 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]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[turip]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (8)

openmeter/customer/httpdriver/customer.go (4)

74-84: ⚠️ Potential issue

Fix duplicate function call and use exported CustomerToAPI

There's a bug in this code where the same conversion function is called twice. The first call (line 74) assigns to item, but then lines 78-81 unnecessarily repeat the same operation, potentially overwriting any previous error.

Apply this fix:

 for _, customer := range resp.Items {
     var item api.Customer

-    item, err = CustomerToAPI(customer)
-    if err != nil {
-        return ListCustomersResponse{}, fmt.Errorf("failed to cast customer customer: %w", err)
-    }
     item, err = CustomerToAPI(customer)
     if err != nil {
         return ListCustomersResponse{}, fmt.Errorf("failed to cast customer customer: %w", err)
     }

     items = append(items, item)
 }

---

142-153: ⚠️ Potential issue

Fix duplicate return statement and use exported CustomerToAPI

The code has a duplicate return statement at line 146 that's outside any function body, which would cause compilation errors. One of these statements should be removed.

Apply this fix:

     return CustomerToAPI(*customer)
 },
 commonhttp.JSONResponseEncoderWithStatus[CreateCustomerResponse](http.StatusCreated),
 httptransport.AppendOptions(
-    return CustomerToAPI(*customer)
-},
-commonhttp.JSONResponseEncoderWithStatus[CreateCustomerResponse](http.StatusCreated),
-httptransport.AppendOptions(
     h.options,
     httptransport.WithOperationName("createCustomer"),
 )...,

---

200-211: ⚠️ Potential issue

Fix duplicate return statement and use exported CustomerToAPI

Similar to the previous issue, there's a duplicate return statement at line 204 that's outside any function body. This would cause compilation errors and needs to be fixed.

Apply this fix:

     return CustomerToAPI(*customer)
 },
 commonhttp.JSONResponseEncoderWithStatus[UpdateCustomerResponse](http.StatusOK),
 httptransport.AppendOptions(
-    return CustomerToAPI(*customer)
-},
-commonhttp.JSONResponseEncoderWithStatus[UpdateCustomerResponse](http.StatusOK),
-httptransport.AppendOptions(
     h.options,
     httptransport.WithOperationName("updateCustomer"),
 )...,

---

276-286: ⚠️ Potential issue

Fix duplicate return statement and use exported CustomerToAPI

The same issue appears again with a duplicate return statement at line 280 that's outside any function body.

Apply this fix:

     return CustomerToAPI(*customer)
 },
 commonhttp.JSONResponseEncoderWithStatus[GetCustomerResponse](http.StatusOK),
 httptransport.AppendOptions(
-    return CustomerToAPI(*customer)
-},
-commonhttp.JSONResponseEncoderWithStatus[GetCustomerResponse](http.StatusOK),
-httptransport.AppendOptions(
     h.options,
     httptransport.WithOperationName("getCustomer"),
 )...,

openmeter/billing/service/invoice.go (3)

242-258: ⚠️ Potential issue

Enhanced GetCustomerOverride with customer expansion

The code now uses the new GetCustomerOverride method with the Expand field set to include customer details. This provides more control over what data is retrieved and makes it explicit what information is being requested.

There appears to be a duplicate error handling block at lines 249-251 that's redundant with the block at 247-248. This could lead to confusion and should be removed:

			customerProfile, err := s.GetCustomerOverride(ctx, billing.GetCustomerOverrideInput{
				Customer: input.Customer,
				Expand:   billing.CustomerOverrideExpand{Customer: true},
			})
			if err != nil {
				return nil, fmt.Errorf("fetching customer profile: %w", err)
-			})
-			if err != nil {
-				return nil, fmt.Errorf("fetching customer profile: %w", err)
			}

---

831-883: ⚠️ Potential issue

Updated invoice update logic to use new customer override API

The invoice update logic now uses the new GetCustomerOverride method and accesses collection configuration through the MergedProfile field. This ensures consistent profile access patterns throughout the codebase.

There appears to be a duplicate error handling block at lines 837-839 that's redundant with the block at 835-836. This should be removed:

		customerProfile, err := s.GetCustomerOverride(ctx, billing.GetCustomerOverrideInput{
			Customer: invoice.CustomerID(),
		})
		if err != nil {
			return billing.Invoice{}, fmt.Errorf("fetching profile: %w", err)
-		}
-		Customer: invoice.CustomerID(),
-		})
-		if err != nil {
-			return billing.Invoice{}, fmt.Errorf("fetching profile: %w", err)
		}

---

974-1019: ⚠️ Potential issue

Simulate invoice updated to use new customer override API and access customer data

The SimulateInvoice method now uses the new GetCustomerOverride method with customer expansion and accesses customer and profile data through the new structure. This provides more explicit control over what data is retrieved.

There appears to be a duplicate error handling block at lines 981-982 that's redundant with the block at 979-980. This should be removed:

	customerProfile, err := s.GetCustomerOverride(ctx, billing.GetCustomerOverrideInput{
		Customer: input.CustomerID,
		Expand:   billing.CustomerOverrideExpand{Customer: true},
	})
	if err != nil {
		return billing.Invoice{}, fmt.Errorf("getting profile with customer override: %w", err)
-	})
-	if err != nil {
-		return billing.Invoice{}, fmt.Errorf("getting profile with customer override: %w", err)
	}

openmeter/billing/adapter/customeroverride.go (1)

281-329: ⚠️ Potential issue

Remove duplicated function definition.

Two versions of GetCustomerOverrideReferencingProfile with identical signatures appear here - lines 281-303 (new) and 305-329 (existing). This duplication likely breaks compilation or leads to ambiguity. Retain the updated version and remove the unused one.

Apply a fix like this by removing the redundant second definition:

-func (a *adapter) GetCustomerOverrideReferencingProfile(ctx context.Context, input billing.HasCustomerOverrideReferencingProfileAdapterInput) ([]customer.CustomerID, error) {
-	return entutils.TransactingRepo(ctx, a, func(ctx context.Context, tx *adapter) ([]customer.CustomerID, error) {
-		dbCustomerOverrides, err := tx.db.BillingCustomerOverride.Query().
-			Where(billingcustomeroverride.Namespace(input.Namespace)).
-			Where(billingcustomeroverride.BillingProfileID(input.ID)).
-			Where(billingcustomeroverride.DeletedAtIsNil()).
-			Select(billingcustomeroverride.FieldCustomerID).
-			All(ctx)
-		if err != nil {
-			return nil, err
-		}
-
-		var customerIDs []customer.CustomerID
-		for _, dbCustomerOverride := range dbCustomerOverrides {
-			customerIDs = append(customerIDs, customer.CustomerID{
-				Namespace: input.Namespace,
-				ID:        dbCustomerOverride.CustomerID,
-			})
-		}
-		return customerIDs, nil
-	})
-}

🧹 Nitpick comments (13)

api/openapi.yaml (5)

419-433: Extension of Parameters in List Endpoint:
The new parameters for filtering customer overrides—specifically, the additions for billing profile filtering, toggling the inclusion of the default profile, expanding response details, and ordering—enhance the endpoint’s flexibility. Ensure that these parameter references (e.g., BillingProfileListCustomerOverridesParams.billingProfile, includeDefaultProfile, and expand) are correctly defined in the components section and consistently used across the API.

---

563-573: Addition of the "expand" Query Parameter:
The introduction of the expand query parameter, which is defined as an array with items referencing the BillingProfileCustomerOverrideExpand schema, clarifies how clients can request additional details in the response. It would be beneficial to include a more detailed description (if not already provided elsewhere) to clearly outline the acceptable values and their effects on the response structure.

---

7318-7352: Detailed Definitions for Query Parameters:
This block defines several query parameters for filtering and expanding customer overrides. The schema for billingProfile as an array of ULIDs is well established, and the boolean parameter includeDefaultProfile (with default true) successfully communicates its purpose. Consider refining the description of includeDefaultProfile for added clarity—for example, “If set to false, only customers explicitly associated with a billing profile override are returned.”

---

8511-8524: Enhanced Response Schema for Customer Overrides:
The addition of the billingProfileWithOverrides and customer properties—using allOf to reference existing schemas—effectively merges related data for client consumption. Verify that the merged response structure aligns with client expectations and that any corresponding documentation is updated to reflect these enhancements.

---

8536-8552: New Schema for CustomerOverrideExpand and OrderBy:
The new BillingProfileCustomerOverrideExpand schema, complete with enumerated values and the x-enum-varnames extension, clearly specifies the expansion options available to clients. Additionally, the introduction of the BillingProfileCustomerOverrideOrderBy schema sets the stage for consistent ordering functionality. Confirm that the allowed enum values and their meanings are clearly communicated in the API documentation and that they align with backend logic.

api/openapi.cloud.yaml (2)

423-427: Ensure Consistent Parameter References for Filtering and Ordering
The added parameter references for filtering (i.e. billingProfile, includeDefaultProfile, and expand) along with the ordering parameters provide the necessary flexibility for the endpoint. Please verify that these parameter names match with those defined elsewhere in the API and are consistently documented.

---

7552-7583: Define New Parameter Schemas for Customer Overrides
The new parameter definitions for BillingProfileListCustomerOverridesParams.billingProfile, expand, and includeDefaultProfile are well structured. The ULID pattern for billing profiles and the boolean default for includeDefaultProfile are appropriate. Consider verifying if the name includeDefaultProfile should be updated to includeAllCustomers for consistency with similar endpoints if that naming convention has been previously adopted.

openmeter/billing/customeroverride.go (3)

202-205: Type aliasing might be confusing.

UpdateCustomerOverrideAdapterInput and CreateCustomerOverrideAdapterInput both point to UpsertCustomerOverrideInput. While this consolidates logic, ensure it doesn’t cause confusion. Adding code comments or docstrings can help.

---

239-245: Clarify array vs. singular naming.

CustomerID is a slice of strings. Consider renaming it to CustomerIDs for more clarity, especially since it accepts multiple IDs. Otherwise, the added fields for extended filtering (name, key, email) are helpful.

---

257-263: Match field name to JSON key.

The field is Customer bool but the JSON tag is "customers,omitempty". This mismatch could cause confusion. Renaming the JSON tag or the field could improve clarity.

api/spec/src/billing/customeroverride.tsp (2)

104-104: Documentation clarity.

The comment on “Customer override values” helps external contributors know the intent of this model. Continue adding such clarifications to the spec.

---

114-114: Workflow override note.

This comment about workflow overrides hints at upcoming expansions. Consider an explicit TODO or open an issue to track anchoring future changes.

openmeter/billing/adapter/customeroverride.go (1)

104-124: Return pattern may cause ambiguity.

When an override isn't found, the method returns (nil, nil) instead of an error. This can introduce ambiguity for callers expecting an explicit “not found” error. Consider returning a dedicated error or handling it similarly to other not-found scenarios for clarity and consistency.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 55556d1 and 0ddea14.

📒 Files selected for processing (64)

• api/client/javascript/src/client/schemas.ts (7 hunks)
• api/openapi.cloud.yaml (5 hunks)
• api/openapi.yaml (5 hunks)
• api/spec/src/billing/customeroverride.tsp (4 hunks)
• openmeter/billing/adapter.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/handler.go (1 hunks)
• openmeter/billing/service.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/customer/adapter/customer.go (1 hunks)
• openmeter/customer/customer.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (4 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/invoice.go (1 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/service/invoice.go (4 hunks)
• openmeter/billing/service/invoiceline.go (1 hunks)
• pkg/lazy/lazy.go (1 hunks)
• api/spec/src/billing/customeroverride.tsp (1 hunks)
• api/openapi.cloud.yaml (11 hunks)
• api/openapi.yaml (11 hunks)
• api/spec/src/billing/customeroverride.tsp (6 hunks)
• api/spec/src/billing/invoices.tsp (0 hunks)
• api/spec/src/billing/profile.tsp (0 hunks)
• openmeter/billing/adapter.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/handler.go (1 hunks)
• openmeter/billing/profile.go (0 hunks)
• openmeter/billing/service.go (1 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/service/invoice.go (12 hunks)
• openmeter/billing/service/invoiceline.go (5 hunks)
• openmeter/billing/validators/subscription/validator.go (2 hunks)
• openmeter/billing/worker/collect/collect.go (2 hunks)
• openmeter/billing/worker/subscription/sync.go (2 hunks)
• openmeter/customer/httpdriver/apimapping.go (1 hunks)
• openmeter/customer/httpdriver/customer.go (4 hunks)
• openmeter/server/router/billing.go (1 hunks)
• test/billing/customeroverride_test.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/service/invoice.go (1 hunks)
• openmeter/customer/adapter.go (1 hunks)
• openmeter/customer/adapter/customer.go (1 hunks)
• openmeter/customer/service.go (1 hunks)
• openmeter/customer/service/customer.go (2 hunks)
• openmeter/server/server_test.go (1 hunks)
• test/billing/customeroverride_test.go (20 hunks)
• test/billing/tax_test.go (4 hunks)
• openmeter/app/stripe/adapter/stripe.go (0 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/customer/customer.go (1 hunks)
• openmeter/customer/httpdriver/customer.go (1 hunks)
• openmeter/productcatalog/subscription/service/create.go (0 hunks)
• test/billing/customeroverride_test.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• test/billing/customeroverride_test.go (1 hunks)

💤 Files with no reviewable changes (5)

• openmeter/billing/profile.go
• openmeter/app/stripe/adapter/stripe.go
• api/spec/src/billing/profile.tsp
• api/spec/src/billing/invoices.tsp
• openmeter/productcatalog/subscription/service/create.go

🔇 Additional comments (107)

pkg/lazy/lazy.go (1)

5-21: Well-implemented thread-safe memoization pattern

The OnceValueErr function provides an elegant way to lazily execute a function once and cache its result, handling errors appropriately. The implementation correctly uses sync.Once to ensure thread safety and properly caches both successful results and errors.

This pattern is particularly useful for expensive operations that might fail but should only be attempted once, like initializing connections or loading configurations.

openmeter/customer/adapter.go (1)

22-22: Good addition to check customer existence

Adding CustomerExists to the interface is a solid improvement that allows for lightweight existence checking without retrieving the entire customer record. This is particularly useful for validations in other services that only need to verify customer existence.

openmeter/customer/service.go (1)

26-26: Service interface properly extended to match adapter capability

The addition of CustomerExists to the service interface aligns with the adapter interface extension, maintaining a consistent API design pattern throughout the application layers.

openmeter/customer/customer.go (1)

123-129: Input validation properly implemented

The added validation method for ListCustomersInput correctly ensures that the namespace field is provided, which is a critical parameter for customer listing operations. This validation helps prevent potential issues and provides clear error messages.

openmeter/customer/httpdriver/apimapping.go (1)

53-54: Good change: Exporting the CustomerToAPI function for reuse.

Making this function public (by capitalizing its name) allows it to be used from other packages, which is consistent with the broader changes to enhance customer API capabilities in this PR. This promotes code reuse and ensures consistent conversion of customer domain objects to API representations across the codebase.

openmeter/server/server_test.go (1)

907-910: LGTM: Added CustomerExists method to NoopCustomerService

This implementation correctly follows the pattern of other methods in the NoopCustomerService struct, implementing the interface method required to support customer existence checks in tests.

openmeter/customer/adapter/customer.go (1)

520-537: Well-structured implementation of CustomerExists

The implementation follows the established patterns in the codebase:

• Uses the transactional repository pattern
• Properly checks for existence with appropriate DB query conditions
• Returns the standard NotFoundError when the customer doesn't exist
• Handles error propagation correctly

openmeter/billing/adapter.go (1)

38-38:

❓ Verification inconclusive

Interface signature change for CreateCustomerOverride

The method signature for creating customer overrides now uses the same input type as updating them, which likely reduces duplication in the codebase. This suggests a consolidation of the creation and update operations for customer overrides.

---

🏁 Script executed:

#!/bin/bash
# Find implementations of this interface to verify they've been updated to match the new signature
ast-grep --pattern 'type $_ struct {
  $$$
}

func ($_ $_) CreateCustomerOverride(ctx context.Context, input $_) ($_, error) {
  $$$
}'

Length of output: 137

---

Manual Verification Required: Update to CreateCustomerOverride Signature

The revised interface now uses the same input type (UpdateCustomerOverrideAdapterInput) for both creating and updating customer overrides, which should reduce duplication in the codebase. However, the automated ast-grep search did not return any results—please verify manually that all implementations of CreateCustomerOverride have been updated to reflect the new signature.

• Location: openmeter/billing/adapter.go, line 38
• Action: Manually inspect implementations of CreateCustomerOverride to ensure they match the new interface signature.

openmeter/customer/service/customer.go (2)

12-12: Added transaction package import

Necessary addition to support the new CustomerExists method's transactional behavior.

---

80-84: Added CustomerExists method to customer service

This implementation properly wraps the adapter's CustomerExists method in a transaction, following the service's responsibility of providing transactional semantics around repository operations.

openmeter/billing/invoice.go (1)

411-416: Good addition of a CustomerID method

This method provides a convenient way to construct a customer.CustomerID object from an Invoice, following the same pattern as the existing InvoiceID method. The implementation correctly uses the invoice's namespace and the customer ID from the invoice's Customer field.

openmeter/billing/httpdriver/handler.go (2)

45-50: Customer override operations look well-defined

The new CustomerOverrideHandler interface follows the established pattern in the codebase for HTTP handlers. The interface properly defines the four essential CRUD operations for customer overrides (list, upsert, get, delete).

---

21-21: Handler interface properly updated

The Handler interface has been correctly updated to include the new CustomerOverrideHandler, ensuring any implementation of Handler will need to support customer override operations.

api/client/javascript/src/client/schemas.ts (7)

2071-2074: Enhanced BillingProfileCustomerOverride schema with context-rich fields

These additions improve the schema by providing more contextual information when retrieving customer overrides - both the overridden billing profile and the customer details are now available.

---

2088-2095: Well-structured enum for controlling response expansion

Good implementation of the expansion mechanism using a string enum. This follows API best practices by allowing clients to request only the data they need, which can improve performance and reduce payload size.

---

7785-7792: Enhanced filtering parameters for customer overrides

The addition of these parameters provides more granular control over the response data. The documentation is clear and helpful, especially for the includeDefaultProfile parameter which explains the behavior difference.

---

7971-7972: Properly exported new type

Correctly exporting the enum type to make it accessible throughout the codebase.

---

8404-8409: Consistent parameter type exports

These exports maintain consistency with the existing pattern in the codebase for parameter types.

---

9027-9038: Enhanced query parameters for the list endpoint

These parameters provide a comprehensive set of filtering and sorting options for the list endpoint. The documentation is clear and follows the same pattern as other endpoint parameters.

---

9129-9131: Added expand parameter to the get endpoint

Good addition of the expand parameter to the get endpoint, providing consistency with the list endpoint and allowing clients to request additional details when needed.

test/billing/tax_test.go (9)

38-38: Variable renaming for clarity.

The variable customer has been renamed to cust, which is consistent with the modifications throughout this test. This change helps avoid confusion with the customer package and is consistently applied throughout the function.

---

73-84: Updated to use new UpsertCustomerOverrideInput structure.

The code has been updated to use the new UpsertCustomerOverrideInput structure instead of a previous creation-specific structure. This is a good change as it unifies the API for both creating and updating customer overrides.

---

86-87: Method updated from CreateCustomerOverride to UpsertCustomerOverride.

The billing service method call has been updated from CreateCustomerOverride to UpsertCustomerOverride, indicating a more flexible operation that can handle both creating new overrides and updating existing ones.

---

89-94: Updated to use new GetCustomerOverrideInput structure with CustomerID.

The updated code now uses the new GetCustomerOverrideInput structure with a customer.CustomerID object that encapsulates both namespace and ID, which is a more structured approach to handling customer identification.

---

96-98: Updated to use MergedProfile property from customer override.

The code now uses customerOverride.MergedProfile to access the profile data, which appears to be a change in how the merged profile data with overrides is accessed. This improves the clarity of what data is being accessed.

---

100-100: Updated variable reference in generateDraftInvoice call.

The call to s.generateDraftInvoice now uses cust instead of the previous variable name, maintaining consistency with the variable renaming throughout the test.

---

107-112: Updated DeleteCustomerOverride to use CustomerID structure.

The code for deleting a customer override has been updated to use the new customer.CustomerID structure, maintaining consistency with the updated API design.

---

115-122: Updated GetCustomerOverride call with consistent CustomerID structure.

This section continues the pattern of updating API calls to use the new customer.CustomerID structure, ensuring consistent handling of customer identifiers throughout the codebase.

---

124-124: Final variable reference update for consistency.

The final call to s.generateDraftInvoice uses the renamed variable cust, completing the consistent application of the variable renaming throughout the test.

openmeter/billing/service.go (2)

35-35: Updated GetCustomerOverride return type to provide merged profile data.

The return type of GetCustomerOverride has been updated from (*CustomerOverride, error) to (*CustomerOverrideWithMergedProfile, error), which enhances the method to return the merged profile data along with the override information. This change aligns with how the data is accessed in the updated test and validation code.

---

37-37: Added new ListCustomerOverrides method to the interface.

A new ListCustomerOverrides method has been added to the CustomerOverrideService interface, enhancing the API with the ability to list customer overrides with pagination support. This is a meaningful addition that complements the existing methods for working with customer overrides.

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

14-14: Added import for customer package.

The customer package has been imported to support the use of the customer.CustomerID structure, which is now used to encapsulate customer identifiers throughout the codebase.

---

85-92: Updated billing service call to use GetCustomerOverride with CustomerID structure.

The billing service call has been updated from GetProfileWithCustomerOverride to GetCustomerOverride with the new input structure that uses a customer.CustomerID object. This change is consistent with the updates in other files and reflects a more structured approach to handling customer identification.

openmeter/billing/validators/subscription/validator.go (4)

9-9: Added import for customer package.

The customer package has been imported to support the use of the customer.CustomerID structure, which is part of the broader refactoring of how customer identifiers are handled.

---

57-66: Updated to use GetCustomerOverride with expanded fields.

The code has been updated to use GetCustomerOverride with the new input structure that includes a customer.CustomerID object and an Expand field to specify that both Apps and Customer should be included in the response. This provides a more structured and explicit approach to retrieving the necessary data.

---

71-71: Updated profile access to use MergedProfile property.

The code now accesses the apps data via customerProfile.MergedProfile.Apps.Invoicing instead of a previous path, reflecting the change in how the merged profile data with overrides is accessed. This improves clarity about the source of the data being used.

---

78-78: Updated ValidateCustomer call to use direct customer reference.

The call to ValidateCustomer now passes customerProfile.Customer directly instead of a pointer, which aligns with the changes in how customer data is structured and accessed in the updated API.

openmeter/server/router/billing.go (4)

15-20: Feature flag implementation for ListBillingProfileCustomerOverrides

The implementation correctly adds conditional logic to check if billing is enabled before proceeding with customer override operations. This is a good practice for feature flagging and helps gracefully handle unimplemented features.

---

26-33: Feature flag implementation for DeleteBillingProfileCustomerOverride

The pattern of checking the BillingEnabled flag before proceeding with operations is consistently applied here. The code correctly calls the unimplemented handler when billing is disabled and returns early.

---

38-47: Updated signature for GetBillingProfileCustomerOverride with expanded parameters

The function signature now includes the params parameter of type api.GetBillingProfileCustomerOverrideParams, which allows for additional options like the Expand field. This change enhances the API's flexibility and is properly passed to the underlying handler.

---

53-60: Feature flag implementation for UpsertBillingProfileCustomerOverride

The implementation follows the same pattern as the other methods, providing consistent behavior across all billing profile customer override operations.

api/openapi.cloud.yaml (3)

567-573: Addition of the 'expand' Query Parameter
Introducing the expand query parameter (with an array schema) is a good enhancement to allow clients to obtain additional details in the response. Ensure that the referenced schema (#/components/schemas/BillingProfileCustomerOverrideExpand) correctly defines the valid expansion values.

---

8663-8670: Enhanced Response with Expanded Details
Adding the billingProfileWithOverrides and customer fields enriches the response when extra details are requested via the expand parameter. Ensure that these fields are only included when applicable to avoid unnecessary payload size, and verify that the referenced schemas for BillingProfile and Customer are up-to-date.

---

8688-8701: New Schema Definitions for Expansion and Ordering
The newly introduced schema for BillingProfileCustomerOverrideExpand with its enum and x-enum-varnames is a solid approach for controlling expansion options. Additionally, make sure that the BillingProfileCustomerOverrideOrderBy enum includes all the expected ordering options so that clients can reliably sort the results.

openmeter/billing/worker/collect/collect.go (2)

104-114: Method call updated to use new customer override API

The code now uses GetCustomerOverride instead of the previous GetProfileWithCustomerOverride method, with a restructured input parameter that encapsulates namespace and ID in a single Customer struct. This change makes the parameter structure more consistent and cohesive.

---

168-177: GetCollectionConfig updated to use new profile access pattern

This change is consistent with the refactoring in the rest of the codebase, now using the GetCustomerOverride method and accessing workflow configuration through the MergedProfile field. This standardizes how customer profiles are retrieved and accessed.

openmeter/billing/service/invoiceline.go (4)

51-51: Type change from ProfileWithCustomerDetails to CustomerOverrideWithDetails

The CustomerProfile field type has been updated to match the new data model. This change aligns with the broader refactoring of how customer profile data is structured and accessed.

---

74-82: Updated profile retrieval with expanded customer details

The code now uses the new GetCustomerOverride method with an Expand field to specify that customer details should be included in the response. This provides more control over what data is retrieved and makes it explicit what information is being requested.

---

209-209: Function signature updated to use new customer profile type

The upsertGatheringInvoiceForCurrency method signature has been updated to use the new billing.CustomerOverrideWithDetails type, maintaining consistency with the rest of the codebase.

---

243-244: Accessing customer and profile data through new structure

The code now accesses customer and profile information through the Customer and MergedProfile fields of the customerProfile object, which is consistent with the new data model.

test/billing/customeroverride_test.go (8)

29-37: Test setup improved with context variable

The test now explicitly creates a context variable and properly sets up a sandbox app and profile before testing. This makes the test setup more clear and ensures proper test isolation.

---

39-48: Test updated to use new customer override API

The test has been updated to use the new GetCustomerOverride method with the restructured input parameter. The assertion has also been changed to check for emptiness rather than nil.

---

58-66: Variable naming improved with cust instead of customer

The variable name has been changed from customer to cust, which helps avoid shadowing the package name and makes the code more readable. This is a good naming practice.

---

93-113: Test enhanced with explicit profile refetching and improved assertions

The test now explicitly refetches the default profile to ensure timestamps are truncated, making the test more robust. The assertions have been updated to compare the BaseProfile instead of the entire profile, which is more precise.

---

213-231: Test updated to check merged profile workflow config

The test now accesses workflow configuration through the MergedProfile field, which is consistent with the new data model. This ensures that the tests verify the correct behavior with the refactored code.

---

295-307: Test updated to check merged profile and customer override

The test now properly checks that when a customer override is deleted, the merged profile falls back to the default profile and the customer override is nil. This ensures that the override deletion behavior works as expected.

---

348-379: Test enhanced with customer refetching and expanded profile retrieval

The test now explicitly refetches the customer to ensure timestamps are truncated and uses the new Expand field to include customer details. The assertion compares the entire customer object, which is more thorough.

---

440-441: Test updated to check customer override collection interval

The test now properly checks that the collection interval in the customer override is nil after updating with null values. This ensures that the behavior of setting null values works as expected.

openmeter/billing/service/invoice.go (2)

128-152: Updated to use new customer override API and access profile data via MergedProfile

The code now uses GetCustomerOverride instead of the previous method and accesses progressive billing configuration through the MergedProfile field. This standardizes how customer profiles are retrieved and accessed.

---

395-402: Updated to access collection configuration through MergedProfile

The code now accesses collection configuration through the MergedProfile field of the customer profile, which is consistent with the new data model.

openmeter/billing/customeroverride.go (10)

12-12: Good use of "lo" for slice utilities.

No issues appear here; the import of github.com/samber/lo is fairly standard and simplifies operations like membership checks.

---

128-137: Confirm optionality of ProfileID.

The new UpsertCustomerOverrideInput struct introduces a ProfileID field without any explicit pointer or null handling. If ProfileID can be empty, ensure the logic that sets it to a default profile meets all requirements. Otherwise, this looks correct.

---

139-162: Validate profile ID if needed.

In the Validate() method, no check is performed on ProfileID. If a valid profile ID is mandatory for certain operations, consider validating it here or clarifying that it's optional. The rest of the required fields and sub-config validations look well handled.

---

214-221: Struct design is clear.

CustomerOverrideWithDetails fosters a clean approach for returning expanded details. The usage of json:",inline" on CustomerOverride is a nice touch, but confirm that fields do not conflict at serialization time.

---

265-269: Useful constant for expansions.

CustomerOverrideExpandAll simplifies expand settings and is a convenient default. No specific concerns here.

---

270-280: Concise order-by type and constants.

Defining CustomerOverrideOrderBy with well-labeled constants is a clean approach. No issues noted.

---

282-288: Explicit set of valid sort fields.

Storing all valid values in CustomerOverrideOrderByValues helps ensure consistent validations. This is well-designed.

---

290-298: Robust validation for order-by values.

The usage of lo.Contains ensures unrecognized values raise an error. This strongly enforces correctness.

---

300-306: Paged response alias is appropriate.

type ListCustomerOverridesResult = pagination.PagedResponse[CustomerOverrideWithDetails] neatly renames the generic structure, improving readability.

---

308-309: Adapter result definition is consistent.

ListCustomerOverridesAdapterResult aligns with the approach of wrapping an override with a customer ID, facilitating consistent pagination in adapters.

api/spec/src/billing/customeroverride.tsp (10)

18-18: Enhanced response detail for listing.

Returning OpenMeter.PaginatedResponse<BillingProfileCustomerOverrideWithDetails> provides more comprehensive override data, aligning with the new model structure.

---

23-46: Shift from create to upsert logic.

Changing @post to @put (implied by the snippet) and adding the | BillingProfileCustomerOverrideWithDetails return type clarifies that the same endpoint can create or update overrides. Verify the client usage to ensure no unexpected collisions.

---

63-63: Detailed override return on GET.

Retrieving BillingProfileCustomerOverrideWithDetails instead of a simpler model ensures consumers have full override context at once.

---

85-89: New order-by fields for more flexible sorting.

Placing customerId, customerName, customerKey, customerPrimaryEmail, and customerCreatedAt in the enum helps the API support richer sorting options. Nice addition.

---

96-96: Enum extension metadata recognized.

@extension("x-enum-varnames", ["all", "apps", "customer"]) is useful for generating well-labeled client code. No issues here.

---

99-99: Explicit mapping for 'apps' option.

apps: "apps" clarifies the expand parameter. This matches the existing style.

---

108-112: Omitting deleted-at from resource timestamps.

...OmitProperties<ResourceTimestamps, "deletedAt"> is consistent with the requirement not to expose deleted resources. Good approach for the spec.

---

126-127: Mandatory customerId ensures clear mapping.

Making customerId required in the model is coherent with the design for scoping overrides. Review related code to confirm all calls pass this field properly.

---

129-158: Introduced BillingCustomerProfile.

This model clarifies how the merged profile is represented after applying overrides. Including references and read-only fields (like apps) is a well-structured approach for expansions.

---

162-188: BillingProfileCustomerOverrideWithDetails for merged data.

The top-level override object now holds both the raw override and the computed merged profile. Ensure each field remains consistent with the new upsert logic in the backend.

openmeter/billing/service/customeroverride.go (26)

5-5: Ensure correct usage of fmt.

Confirm that the added fmt import is only for formatting error messages or logs. No issues noted otherwise.

---

16-18: New Upsert behavior returns CustomerOverrideWithDetails.

Replacing the previous create method with an upsert approach clarifies the combined creation/update flow. The def variable helps unify default returns in error paths.

---

20-20: Return validation error with more context.

Wrapping the original error in billing.ValidationError is helpful for client-side checks. This fosters consistent error handling.

---

25-25: Transactional approach is sound.

transaction.Run ensures atomic updates for retrieving and creating or updating overrides. This is a good design for concurrency and rollback safety.

---

35-35: Short-circuit with default object.

Returning def on error ensures a consistent fallback object in early returns while clarifying that no partial override is returned.

---

44-44: Handle adapter errors gracefully.

Again, returning def plus the error indicates a clear path for context failures. Good error-handling pattern.

---

48-48: Surface default-profile not-found errors.

Raising a billing.NotFoundError encourages clients to handle the missing default profile. This reveals a well-defined error boundary.

---

57-69: Consolidated update/create logic.

The snippet merges “update” and “create” paths under the same input struct, minimizing code duplication. Ensure all fields are handled consistently during each flow.

---

71-71: Resolve details after upsert.

Returning resolveCustomerOverrideWithDetails ensures the immediate response includes both raw and merged profile data. Good approach for up-to-date results.

---

80-80: Error propagation in transaction.

Forwarding errors from the adapter maintains consistency. No concerns here.

---

83-83: Clean final return.

Returning adapterOverride allows the outer function to produce the final merged result. Looks correct.

---

86-88: Return CustomerOverrideWithDetails for GET.

Switching from a pointer-based simple override to CustomerOverrideWithDetails ensures the client obtains the merged configuration in one request.

---

90-90: Consistent validation error object.

Again, good usage of billing.ValidationError. Reinforces the pattern established in upsert.

---

101-101: Surface retrieval errors properly.

Protects the code from returning partial or nil override data. Safe design.

---

105-105: 404 upon missing override.

billing.NotFoundError is the correct approach when no override was found for the given customer. Good practice.

---

112-124: Handle default profile fallback path.

When the existing override lacks a profile, retrieving and assigning the default profile is consistent with the new approach. The error checks are clear and thorough.

---

139-139: List method gracefully extends overrides.

ListCustomerOverrides now returns a ListCustomerOverridesResult built upon the updated override structure. This unifies the approach with the new upsert and get patterns.

---

184-184: Avoid repeated customer lookups.

Pulling customers in bulk helps performance and avoids concurrency pitfalls or API spam. This design is well-considered.

---

185-197: Consolidated retrieval with expanded customers.

Mapping override records to their associated customers using a single query is an efficient approach, especially for large result sets.

---

201-201: Introducing resolveCustomerOverrideWithDetailsInput.

Encapsulating parameters for the resolution logic fosters reusability and keeps the calling code cleaner.

---

205-215: Default profile retrieval for list operations.

Checking once whether any override lacks a baseline profile avoids repeated default-profile lookups. Ensures consistent fallback logic.

---

218-231: Bulk-mapping overrides to the final view.

pagination.MapPagedResponseError transforms each item into CustomerOverrideWithDetails with minimal duplication. This pattern is neatly compatible with the existing pagination framework.

---

234-251: Helper for retrieving from a cache.

GetCustomerFromCache avoids additional DB calls and clarifies how expansions for Customer are resolved. This is a straight-forward approach.

---

252-260: Optional direct default profile retrieval.

GetDefaultProfile from resolveCustomerOverrideWithDetailsInput centralizes fallback logic. No major issues here.

---

262-270: Validation hook ensures consistent input.

Revalidating the CustomerID ensures we never proceed with ill-formed data. Reporting a billing.ValidationError again centralizes error handling.

---

272-308: Modular resolution of merged profiles.

Splitting the resolution logic into resolveProfileWorkflow and resolveProfileApps clarifies responsibilities and reduces duplication in the main service flow. Merging overrides with baseline profiles here is well-done.

openmeter/billing/adapter/customeroverride.go (1)

78-79: Confirm intent to restore a soft-deleted record.

Calling ClearDeletedAt() here will effectively un-delete a previously deleted override. Ensure this is the desired behavior when updating an override.

Do you want me to check related code references or create usage verification scripts to confirm this logic?

[coderabbitai]

Actionable comments posted: 5

🔭 Outside diff range comments (4)

openmeter/customer/httpdriver/customer.go (4)

74-81: ⚠️ Potential issue

Remove duplicated code: CustomerToAPI is called twice

There's a duplicate call to CustomerToAPI on lines 74 and 78, with both trying to handle error checks on the same conversion. This would cause the second call to needlessly overwrite the result of the first call.

Apply this diff to fix the issue:

-				item, err = CustomerToAPI(customer)
-				if err != nil {
-					return ListCustomersResponse{}, fmt.Errorf("failed to cast customer customer: %w", err)
-				}
-				item, err = CustomerToAPI(customer)
-				if err != nil {
-					return ListCustomersResponse{}, fmt.Errorf("failed to cast customer customer: %w", err)
-				}
+				item, err = CustomerToAPI(customer)
+				if err != nil {
+					return ListCustomersResponse{}, fmt.Errorf("failed to cast customer customer: %w", err)
+				}

---

146-147: ⚠️ Potential issue

Remove duplicated code: Orphaned return statement

There's a duplicated return statement that appears to be an artifact from code restructuring. This line should be removed.

Apply this diff to fix the issue:

-			return CustomerToAPI(*customer)
-		},

---

204-205: ⚠️ Potential issue

Remove duplicated code: Orphaned return statement

There's a duplicated return statement that appears to be an artifact from code restructuring. This line should be removed.

Apply this diff to fix the issue:

-			return CustomerToAPI(*customer)
-		},

---

280-281: ⚠️ Potential issue

Remove duplicated code: Orphaned return statement

There's a duplicated return statement that appears to be an artifact from code restructuring. This line should be removed.

Apply this diff to fix the issue:

-			return CustomerToAPI(*customer)
-		},

🧹 Nitpick comments (18)

api/openapi.cloud.yaml (3)

7559-7600: Parameter Definitions for Customer Override Filtering

The detailed parameter definitions for BillingProfileListCustomerOverridesParams.customerId, customerKey, customerName, and customerPrimaryEmail are clearly specified with appropriate validation (patterns and examples). Note that customerId is defined as an array—if the intention is to support multiple values, this is correct; otherwise, consider using a single string type.

---

8347-8383: Introduction of BillingCustomerProfile Schema

The new BillingCustomerProfile schema, which now includes required fields such as supplier, workflow, and apps, provides a structured way to represent customer-specific merged billing profiles. Consider providing examples or additional constraints to further enhance its documentation and ensure comprehensive validation.

---

8739-8791: Refined Enumerations and Detailed Customer Override Structure

The enriched enumerations in BillingProfileCustomerOverrideOrderBy (including values such as customer.Id, customerName, etc.) and the detailed BillingProfileCustomerOverrideWithDetails schema improve the API's flexibility and expressiveness. Please confirm that the capitalization in enum values (e.g., customer.Id) is intentional for consistency with related specifications.

api/openapi.yaml (1)

496-502: Review Response Schema Using anyOf

The change introduces an anyOf for the response schema, allowing results to be of either BillingProfileCustomerOverride or BillingProfileCustomerOverrideWithDetails. This is a flexible solution for backwards compatibility. It would be good to update your API documentation so that consumers are aware of the dual response type.

openmeter/billing/service/invoiceline.go (2)

51-51: Clarify the field naming.

Renaming CustomerProfile to something like OverrideWithDetails could help distinguish overrides from standard profiles.

---

243-244: Defensive null check suggested.

Calling lo.FromPtr(customerProfile.Customer) presumes non-nil. If customerProfile.Customer is ever nil, consider graceful handling or validations before assignment.

test/billing/customeroverride_test.go (5)

35-36: Validate test coverage.

Creating a billing profile without further assertions may reduce clarity. Consider confirming the returned profile’s fields as well.

---

58-58: Customer creation logic.

Creating the new customer is fine; consider verifying the returned ID matches expectations as an additional assertion.

---

71-75: Check default profile fallback.

When retrieving a customer override via GetCustomerOverride, verify if fallback logic to default profiles is thoroughly tested when none is found.

---

122-122: Detailed override config.

The override additions (Collection, Invoicing, Payment) are well-defined. Ensure complete coverage in separate test cases if needed.

---

149-149: Workflow config references.

The reference to wfConfig is consistent; double-check that all relevant fields are asserted for correctness.

openmeter/billing/service/customeroverride.go (3)

25-25: Use transactions carefully.

Wrapping the upsert steps in transaction.Run is excellent. Verify that partial failures are correctly rolled back and retried if necessary.

---

115-122: Fallback to default profile.

When Profile is nil in the override, falling back to a default profile aligns with the new approach. Verify that no infinite loops can occur if default profile is also missing.

---

272-307: Ensure merger logic is well-tested.

The resolveProfileWorkflow merges the baseline profile and override. Validate all edge cases—e.g., partial override fields, deleted profiles, or concurrency concerns—are tested.

openmeter/billing/customeroverride.go (2)

139-139: Add unit tests for the Validate method.
Validation looks sound, but ensure coverage with test cases for the new upsert scenario (empty Namespace, empty CustomerID, etc.).

---

239-245: Clarify filter matching.
These new fields (CustomerName, CustomerKey, etc.) are excellent for filtering, but ensure the API clarifies whether matching is exact or partial, and if it’s case-sensitive.

api/spec/src/billing/customeroverride.tsp (1)

185-244: Added query params for more filtering.
Great for flexible queries, but document whether partial text or exact match is expected.

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

28-91: List handler logic is sound but consider error context.
The usage of resolveNamespace and lo.FromPtr is clean and merges user inputs well. If an error occurs while mapping overrides, consider adding more context to help diagnose issues.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 55556d1 and 0ddea14.

📒 Files selected for processing (64)

• api/client/javascript/src/client/schemas.ts (7 hunks)
• api/openapi.cloud.yaml (5 hunks)
• api/openapi.yaml (5 hunks)
• api/spec/src/billing/customeroverride.tsp (4 hunks)
• openmeter/billing/adapter.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/handler.go (1 hunks)
• openmeter/billing/service.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/customer/adapter/customer.go (1 hunks)
• openmeter/customer/customer.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (4 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/invoice.go (1 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/service/invoice.go (4 hunks)
• openmeter/billing/service/invoiceline.go (1 hunks)
• pkg/lazy/lazy.go (1 hunks)
• api/spec/src/billing/customeroverride.tsp (1 hunks)
• api/openapi.cloud.yaml (11 hunks)
• api/openapi.yaml (11 hunks)
• api/spec/src/billing/customeroverride.tsp (6 hunks)
• api/spec/src/billing/invoices.tsp (0 hunks)
• api/spec/src/billing/profile.tsp (0 hunks)
• openmeter/billing/adapter.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/handler.go (1 hunks)
• openmeter/billing/profile.go (0 hunks)
• openmeter/billing/service.go (1 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/service/invoice.go (12 hunks)
• openmeter/billing/service/invoiceline.go (5 hunks)
• openmeter/billing/validators/subscription/validator.go (2 hunks)
• openmeter/billing/worker/collect/collect.go (2 hunks)
• openmeter/billing/worker/subscription/sync.go (2 hunks)
• openmeter/customer/httpdriver/apimapping.go (1 hunks)
• openmeter/customer/httpdriver/customer.go (4 hunks)
• openmeter/server/router/billing.go (1 hunks)
• test/billing/customeroverride_test.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/service/invoice.go (1 hunks)
• openmeter/customer/adapter.go (1 hunks)
• openmeter/customer/adapter/customer.go (1 hunks)
• openmeter/customer/service.go (1 hunks)
• openmeter/customer/service/customer.go (2 hunks)
• openmeter/server/server_test.go (1 hunks)
• test/billing/customeroverride_test.go (20 hunks)
• test/billing/tax_test.go (4 hunks)
• openmeter/app/stripe/adapter/stripe.go (0 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/customer/customer.go (1 hunks)
• openmeter/customer/httpdriver/customer.go (1 hunks)
• openmeter/productcatalog/subscription/service/create.go (0 hunks)
• test/billing/customeroverride_test.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• test/billing/customeroverride_test.go (1 hunks)

💤 Files with no reviewable changes (5)

• openmeter/productcatalog/subscription/service/create.go
• openmeter/billing/profile.go
• openmeter/app/stripe/adapter/stripe.go
• api/spec/src/billing/profile.tsp
• api/spec/src/billing/invoices.tsp

🔇 Additional comments (119)

pkg/lazy/lazy.go (1)

5-21: Well-implemented lazy initialization with error handling

This is a clean implementation of the lazy initialization pattern that properly handles errors. The code correctly uses sync.Once to ensure thread safety when initializing the value, making it suitable for concurrent usage.

The function signature and documentation clearly communicate the intended behavior, and the generic implementation provides type safety while maintaining flexibility.

openmeter/customer/service.go (1)

26-26: LGTM: Clean method addition to the interface

The CustomerExists method is a well-defined addition to the CustomerService interface, following the consistent pattern of taking a context and customer identifier and returning an error.

openmeter/customer/adapter.go (1)

22-22: LGTM: Consistent interface method addition

The CustomerExists method added to the CustomerAdapter interface mirrors the addition to the CustomerService interface, maintaining the architectural pattern of the system.

openmeter/billing/invoice.go (1)

411-416: LGTM: Well-implemented CustomerID accessor method

This method provides a clean way to get a structured CustomerID from an Invoice, following the same pattern as the existing InvoiceID() method. This will help simplify code that needs to check customer existence from an invoice context.

openmeter/server/server_test.go (1)

908-910: LGTM: Appropriate implementation for testing service

The CustomerExists implementation for NoopCustomerService is correctly implemented as a no-op method that always returns nil, consistent with the pattern of this test service.

openmeter/customer/customer.go (1)

123-129: Input validation looks good!

The Validate() method added to ListCustomersInput follows the same pattern as other validation methods in this file. It correctly checks for a required field and returns an appropriate validation error when needed.

openmeter/billing/httpdriver/handler.go (2)

21-21: Handler interface update is appropriate

Adding the CustomerOverrideHandler interface to the main Handler interface follows the established pattern in this file for organizing handler functionality.

---

46-51: New CustomerOverrideHandler interface looks good

The new interface provides a comprehensive set of methods for managing customer overrides (list, upsert, get, delete) consistent with the patterns used for other entities in this file.

openmeter/customer/service/customer.go (2)

12-12: Import organization is appropriate

The added import for the transaction package is necessary for the new CustomerExists method.

---

80-84: CustomerExists implementation looks good

The method properly uses transaction handling and delegates to the adapter layer, consistent with the service's architectural pattern.

openmeter/customer/httpdriver/apimapping.go (1)

53-54: Function visibility change is appropriate

Changing customerToAPI to CustomerToAPI to make it publicly accessible is a good approach if this mapping function needs to be reused across packages. This change supports the customer override API functionality being added in this PR.

openmeter/customer/adapter/customer.go (1)

520-537: Solid implementation of CustomerExists method.

The method correctly checks for customer existence within a transaction, ensuring only non-deleted customers are considered and returning appropriate errors. This implementation aligns well with the existing patterns in the codebase.

api/client/javascript/src/client/schemas.ts (6)

2071-2074: New fields added to BillingProfileCustomerOverride

The addition of billingProfileWithOverrides and customer fields enhances the detail available in customer override responses. These fields allow for including the merged billing profile with customer-specific overrides and the associated customer information.

---

2088-2095: New enum for expanding resources

The introduction of BillingProfileCustomerOverrideExpand follows API best practices by allowing clients to specify which related resources should be included in the response. This supports more efficient API usage where clients can request only the data they need.

---

7971-7972: Export for the new expand enum

Properly exporting the BillingProfileCustomerOverrideExpand type ensures it's available for use throughout the codebase.

---

8404-8409: Export of parameter types for TypeScript type safety

The export of these parameter types ensures proper type checking when using the API client.

---

9027-9038: Enhanced query parameters for listing customer overrides

The implementation of these query parameters provides comprehensive filtering and sorting capabilities for the listBillingProfileCustomerOverrides endpoint. The inclusion of ordering parameters (order and orderBy) alongside the new filtering options creates a consistent and flexible API design.

---

9129-9131: Added expand capability to single override retrieval

Adding the expand parameter to the getBillingProfileCustomerOverride endpoint provides consistency with the list endpoint and allows clients to control the level of detail in the response.

openmeter/billing/adapter.go (1)

38-38: Input type alignment for CreateCustomerOverride

The method signature has been updated to use UpdateCustomerOverrideAdapterInput instead of the previous CreateCustomerOverrideInput, standardizing the input type between creation and update operations. This aligns with the upsert pattern adopted elsewhere in the codebase.

openmeter/customer/httpdriver/customer.go (3)

142-142: Method renamed from private to public

The function has been correctly renamed from customerToAPI to CustomerToAPI, making it public and consistent with its usage throughout the codebase.

---

200-201: Method renamed from private to public

The function has been correctly renamed from customerToAPI to CustomerToAPI, making it public and consistent with its usage throughout the codebase.

---

276-277: Method renamed from private to public

The function has been correctly renamed from customerToAPI to CustomerToAPI, making it public and consistent with its usage throughout the codebase.

openmeter/billing/validators/subscription/validator.go (4)

7-9: Updated imports order

The import for the customer package has been correctly added and organized.

---

57-66: API update for customer override retrieval

The method call has been updated from GetProfileWithCustomerOverride to GetCustomerOverride with a new input structure that uses a dedicated CustomerID type and explicit expansion options. This enhances type safety and makes it clearer what related data is being included.

---

71-71: Updated profile access pattern

Changed from customerProfile.Profile.Apps.Invoicing to customerProfile.MergedProfile.Apps.Invoicing, indicating a new approach to handling merged or overridden customer profile data.

---

78-78: Changed parameter passing

Updated to pass customerProfile.Customer directly instead of a pointer, aligning with the new data structure returned by GetCustomerOverride.

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

14-14: Added customer package import

The import for the customer package has been correctly added to support the updated method call.

---

85-92: API update for customer override retrieval

The method call has been updated from GetProfileWithCustomerOverride to GetCustomerOverride with a new input structure that uses a dedicated CustomerID type. This enhances type safety by encapsulating both namespace and ID in a single type.

test/billing/tax_test.go (6)

38-38: Consistent variable renaming

The variable has been renamed from customer to cust, which is then consistently used throughout the rest of the method. This maintains code clarity while being more concise.

---

73-84: API update to use UpsertCustomerOverrideInput

The code has been updated to use UpsertCustomerOverrideInput instead of CreateCustomerOverrideInput, reflecting the consolidation of create and update operations into a single upsert method. The structure also properly uses the customer ID from the new cust variable.

---

86-94: Updated to use UpsertCustomerOverride method

The method call has been updated to use UpsertCustomerOverride instead of CreateCustomerOverride, consistent with the API changes. The subsequent GetCustomerOverride call now uses a structured input with a proper Customer type containing namespace and ID.

---

96-98: Updated reference to use MergedProfile

The assertions now correctly reference the MergedProfile property, which aligns with the updated return type of the customer override API that now provides more detailed information.

---

108-111: Consistent use of structured CustomerID

The deletion operation now uses a properly structured CustomerID with both namespace and ID fields, making the API more consistent and type-safe.

---

115-122: Consistent use of GetCustomerOverride

The GetCustomerOverride call and its assertions are correctly updated to use the structured customer ID and reference the merged profile in the response, maintaining consistency with the updated API design.

openmeter/server/router/billing.go (4)

15-21: Added proper billing feature check

The code now correctly checks if billing is enabled before proceeding with the request handling. If billing is not enabled, it delegates to an unimplemented handler, providing a consistent pattern for feature toggling.

---

26-34: Consistent implementation of feature check for DeleteCustomerOverride

The same pattern for checking if billing is enabled is applied to the delete operation, maintaining consistency across all billing-related endpoints.

---

38-48: Updated method signature with expanded parameters

The method signature has been updated to include additional parameters for fetching customer overrides, enabling more flexible queries. The implementation correctly passes the expand parameter to the underlying handler, enhancing the API's functionality.

---

53-61: Consistent implementation of feature check for UpsertCustomerOverride

The upsert operation follows the same pattern for feature checking and parameter handling, ensuring consistency across the API surface.

api/openapi.cloud.yaml (8)

421-443: Expanded Filtering Parameters for List Customer Overrides

The addition of new parameters (includeAllCustomers, customerId, customerName, customerKey, and customerPrimaryEmail) enhances the filtering capabilities for listing customer overrides. Please verify that the descriptions clearly communicate each parameter’s intent and that using an array for customerId is intentional (i.e., to support filtering by multiple customer IDs).

---

477-483: New PUT Endpoint for Customer Override Upsert

The introduction of the PUT method with the operationId upsertBillingProfileCustomerOverride under the /api/v1/billing/customers/{customerId} endpoint is clear and aligns with the API’s design goals. Ensure that the path parameter (customerId) and associated validations are correctly handled in the implementation.

---

497-503: Enhanced Response Schema for Creation

Expanding the response schema’s anyOf to include a reference to BillingProfileCustomerOverrideWithDetails allows the API to return a detailed customer override when needed while preserving backward compatibility. Confirm that existing clients can support both response formats as intended.

---

579-585: Consistent Success Response Schema

Referring to the BillingProfileCustomerOverrideWithDetails schema in the JSON response for successful requests (HTTP 200/201) ensures that detailed override information is consistently delivered. This change promotes clarity and consistency across the API responses.

---

7604-7619: Updated includeAllCustomers Parameter

Renaming the parameter from includeDefaultProfile to includeAllCustomers, along with the updated multi-line description, improves clarity regarding its functionality. This provides better guidance to API consumers on how customer overrides affect the returned result set.

---

8693-8697: Enforcing Customer Identity in Schema

Adding customerId as a required field in this schema reinforces data integrity by ensuring that every customer override record is explicitly associated with a customer. This is a beneficial change for maintaining consistent records.

---

8709-8725: Detailed Override Schema Enhancements

Introducing the billingProfileId (with guidance on dynamic lookup if empty) alongside the customerId property in the override schema clarifies the relationship between customer overrides and billing profiles. The use of patterns and examples enhances schema validation.

---

8808-8816: Updated Paginated Response Schema

Referencing BillingProfileCustomerOverrideWithDetails within the paginated response schema ensures that API consumers receive rich, detailed customer override data consistently. Verify that the overall pagination structure aligns with the rest of the API design for a seamless developer experience.

openmeter/billing/adapter/customeroverride.go (4)

3-21: Import addition for enhanced filtering capabilities.

The addition of the slices package and the updated import of predicate indicate preparations for the new filtering approach in the ListCustomerOverrides function.

---

193-197: Good refactoring: Consolidated customer override filters into a reusable slice.

The creation of a customerOverrideFilters slice improves code organization and reusability throughout this function.

---

199-201: Well-structured conditional filter extension based on billing profiles.

This addition correctly extends the filter set when billing profiles are specified, making the query more flexible.

---

203-233: Improved customer filtering logic with default profile handling.

This enhanced implementation provides better control over which customers are included in query results:

1. For non-inclusive queries, it simply applies the filters directly
2. For inclusive queries, it determines whether the default profile should be included based on provided parameters
3. When including the default profile, it properly uses an OR condition to include customers without overrides

The explicit consideration of the default profile makes the API more powerful for client use cases.

openmeter/billing/service/invoice.go (10)

128-134: Method refactoring: GetProfileWithCustomerOverride → GetCustomerOverride.

The change to use GetCustomerOverride follows a service-wide refactoring pattern for better data organization. The method now returns a combined structure containing both customer override settings and the profile.

---

151-152: Updated access path for profile properties.

Following the method signature change, the code now correctly accesses the progressive billing setting through MergedProfile instead of directly from Profile.

---

242-245: Additional expand parameter added to GetCustomerOverride call.

The addition of the Expand parameter with Customer: true ensures that the customer details are included in the response, which is necessary for the invoice processing.

---

257-258: Consistent access path update for progressive billing configuration.

This change maintains consistency with other access pattern changes in the file by using customerProfile.MergedProfile.

---

292-293: Updated reference to customer name through merged profile.

The sequence generation now correctly references the customer name from the merged profile data structure.

---

304-306: Enhanced invoice creation with customer and profile data.

The invoice creation now uses the full customer object and merged profile data, providing more complete information for the invoice.

---

865-872: Updated collection time adjustment with merged profile reference.

The update to use customerProfile.MergedProfile maintains consistency with other changes throughout the file when referencing collection interval configuration.

---

883-884: Consistent reference to progressive billing configuration.

This change ensures that the progressive billing setting is accessed consistently from the merged profile structure.

---

974-977: Method call updated with expanded customer information.

The addition of the Expand parameter with Customer:true ensures the SimulateInvoice function has access to all necessary customer details.

---

1002-1019: Comprehensive update to reference customer and supplier properties.

All properties are now correctly accessed through the new structure with appropriate path adjustments:

• Customer details from customerProfile.Customer
• Supplier and workflow details from customerProfile.MergedProfile

This ensures the invoice contains complete and accurate information.

openmeter/billing/worker/collect/collect.go (4)

104-109: Updated customer profile retrieval with structured input.

The method now uses a properly structured GetCustomerOverrideInput with a customer.CustomerID object containing both namespace and ID fields, making the API more coherent.

---

114-115: Updated profile property access pattern.

The code now correctly calculates the asOf time using the interval from the merged profile's workflow configuration, maintaining consistency with other service changes.

---

168-170: Simplified GetCollectionConfig method with proper input structure.

The method now uses the more concise CustomerID structure, making the API simpler to use and maintaining consistency across the codebase.

---

177-178: Consistent access to merged profile configuration.

The function now returns the collection configuration from the merged profile, ensuring it contains the properly resolved settings considering both default and override values.

api/openapi.yaml (10)

420-431: Validate Updated Query Parameters for Customer Overrides

The new filtering parameters (i.e. includeAllCustomers, customerId, customerName, customerKey, and customerPrimaryEmail) in the list endpoint are clearly added and structured. Please verify that these references correctly match the defined components in your schema and that the naming is consistent across documentation and client usage.

---

476-482: Confirm New Upsert Endpoint Definition

The newly added put method on /api/v1/billing/customers/{customerId} for upserting billing profile customer overrides is well-defined with a clear operationId and description. Please ensure that corresponding backend logic and tests are updated.

---

578-584: Approve Detailed Response Schema for Customer Override

The update to reference BillingProfileCustomerOverrideWithDetails in the successful response (200) enhances the payload detail. Ensure that this update is consistently reflected in client docs and that tests exercise the new structure.

---

7325-7365: Verify New Customer Filter Parameter Schemas

New parameter definitions for BillingProfileListCustomerOverridesParams.customerId, .customerKey, .customerName, and .customerPrimaryEmail are introduced with clear descriptions and examples. One point to confirm: the customerId parameter is defined as an array of strings with a ULID pattern. Please verify that this is the intended design (i.e. supporting multiple customer IDs) rather than a single identifier.

---

7370-7385: Ensure Clarity in includeAllCustomers Description

Renaming from includeDefaultProfile to includeAllCustomers improves clarity regarding functionality. The provided multi-line description is helpful; however, consider simplifying the wording if possible to improve readability for API consumers.

---

8198-8232: Review New BillingCustomerProfile Object Definition

The newly introduced BillingCustomerProfile object with required properties (supplier, workflow, and apps) and detailed descriptions provides a clearer model for a merged customer profile. Ensure that any existing integrations consuming this definition are updated accordingly.

---

8544-8550: Assess Backward Compatibility for BillingLineCharge Changes

Adding customerId as a required field in the BillingLineCharge object is a significant update. Please double-check that this change is backwards-compatible and that any systems using this schema are adjusted to include the new field.

---

8560-8571: Approve Additional Fields in Customer Override Object

The inclusion of billingProfileId (with dynamic lookup if empty) and customerId in the customer override schema adds clarity on associations. Ensure that API consumers and backend processing logic are informed of these new, detailed association fields.

---

8590-8609: Confirm Updated OrderBy Enum and Details Object

The update of the BillingProfileCustomerOverrideOrderBy enum to include new order options (e.g. customer.Id, customerName, etc.) and the introduction of the BillingProfileCustomerOverrideWithDetails object (which mandates baseBillingProfileId) align with the enhanced response structure. Please confirm that client integrations handle these new mandatory fields correctly.

---

8659-8665: Approve Paginated Response Schema Update

Updating the BillingProfileCustomerOverrideWithDetailsPaginatedResponse to use the detailed override schema in its items is an excellent move for consistency and clarity in responses. Ensure that tests validate this updated structure.

openmeter/billing/service.go (2)

33-33: Adopt the new Upsert approach gracefully.

Introducing an UpsertCustomerOverride method is a clean way to consolidate separate create/update paths. Ensure you’ve removed or refactored all previous calls to any old create/update methods so clients always use this new method.

---

36-37: Keep method signatures consistent.

Switching to return CustomerOverrideWithDetails is consistent with the new approach. Ensure all calling sites handle the non-pointer return correctly, especially regarding nil checks.

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

74-81: Validate expand usage.

Expanding customer details is beneficial for richer data. Verify that upstream callers handle a potentially larger payload and consider test coverage for both expanded and unexpanded scenarios.

---

209-209: Confirm signature changes are reflected elsewhere.

Changing the function to accept billing.CustomerOverrideWithDetails might break old references. Double-check test coverage and any dependent code that may still expect the previous signature.

---

217-217: Namespace update appears correct.

Setting Namespaces: []string{input.Namespace} is consistent with scoping. No issues here.

test/billing/customeroverride_test.go (11)

29-30: Context usage is appropriate.

Using a background context is standard practice for tests. No concerns here.

---

31-33: Sandbox setup logic is clear.

Installing the sandbox app and setting up the profile input is straightforward. Looks good.

---

39-39: Error handling logic is solid.

Fetching a non-existing user override should yield a not-found error. The approach is correct for negative test coverage.

---

48-48: Empty assertion is correct.

Ensuring customerEntity is empty properly confirms no override object is returned.

---

65-67: Good use of require checks.

Ensuring no error and retrieving a non-nil customer are helpful correctness guards.

---

79-80: Asserting error type.

require.ErrorAs correctly enforces the expected billing NotFoundError, followed by an empty override check.

---

113-113: Truncating timestamps.

Re-fetching the default profile to ensure truncated timestamps is a good approach for test reliability.

---

117-118: Explicit error checks.

Using require.NoError then require.NotNil clarifies test outcomes for the default profile retrieval.

---

137-138: Proper verification steps.

Validating auto-advance and draft period logic ensures that the pinned profile override is respected.

---

200-201: Deleting then re-creating override.

The test thoroughly ensures that a deleted override no longer appears, then re-verifies creation logic. This is robust coverage.

---

214-214: Confirm final override state.

Asserting that customerProfile.CustomerOverride is nil correctly checks the override was deleted or never existed. Good coverage.

openmeter/billing/service/customeroverride.go (3)

16-18: Consolidate create/update into Upsert.

Combining the logic into UpsertCustomerOverride is a neat improvement. Ensure you have handled any external references to the old create/update functions to avoid breakages.

---

20-20: Return structure aids clarity.

Returning a default struct instead of nil helps avoid accidental nil-pointer accesses at call sites. Great approach.

---

71-78: Override resolution block is straightforward.

The logic merges an existing override or creates a new one, then resolves with details. Confirm all error paths have test coverage, especially partial override updates.

openmeter/billing/customeroverride.go (8)

12-12: No issues with the new import.
Importing github.com/samber/lo is a good choice for functional utilities and list/mapper methods.

---

128-128: Consider concurrency or version checks when upserting.
The new UpsertCustomerOverrideInput type no longer includes an UpdatedAt or version field. If multiple updates could conflict, consider including an optimistic concurrency mechanism (e.g., version number).

---

202-205: Aliases reduce duplicate code.
Type aliases (UpdateCustomerOverrideAdapterInput and CreateCustomerOverrideAdapterInput) nicely consolidate logic.

---

214-220: Be cautious with json:",inline" usage.
Inlining CustomerOverride might cause naming collisions if fields overlap. Also consider nil-pointer checks to avoid panics when CustomerOverride is nil.

---

265-268: Default expansions look correct.
Defining CustomerOverrideExpandAll provides a convenient way to include all expansions.

---

270-288: Enum-based ordering is well-structured.
The constants and slice of allowed values are clear and robust for ordering.

---

290-301: Validation of order-by field looks good.
Using lo.Contains() to confirm validity is straightforward.

---

302-309: Inline references are fine but watch for collisions.
CustomerOverrideWithCustomerID inlines *CustomerOverride. Confirm that any overlapping JSON fields are handled properly.

api/spec/src/billing/customeroverride.tsp (8)

18-18: Switch to detailed paginated response is appropriate.
Returning BillingProfileCustomerOverrideWithDetails enables richer information.

---

23-23: HTTP method changed to PUT for upserts.
This more accurately reflects idempotent upsert behavior.

---

46-46: Return detailed override on success.
Providing BillingProfileCustomerOverrideWithDetails is beneficial for the client.

---

63-63: Get method now returns the new detailed model.
Allows the client to see merged billing profile data.

---

85-90: Ensure these enum values match the schema.
Verify that "customer.Id", "customerName", etc. align with your DB or domain naming.

---

96-96: Enum varnames extension is fine.
["all", "apps", "customer"] enumerations are consistent with the code references.

---

104-104: Enhanced documentation for customer override.
Short doc comment updates typically don’t pose issues.

---

108-111: Omitting deletedAt from the override is clear.
Removes confusion since the override listing doesn't consider deleted records.

openmeter/billing/httpdriver/customeroverride.go (10)

4-9: New imports are appropriate.
Imports like context, net/http, and lo support your new logic.

---

12-19: Additional imports for service interactions.
No conflicts or duplications spotted.

---

23-26: Refined type definitions for list requests and responses.
Switching to the api.BillingProfileCustomerOverrideWithDetailsPaginatedResponse type standardizes the format.

---

93-101: Typed request/response for “Get” is well-structured.
Separating GetCustomerOverrideRequest and GetCustomerOverrideParams clarifies handler logic.

---

103-144: Get handler provides a consistent pattern.
Namespace resolution and mapping expansions is straightforward. Ensure robust error handling (e.g. 404 vs. 500).

---

146-153: Defines upsert types properly.
Reusing existing structures with typed parameters clarifies your approach.

---

193-201: Delete type definitions are straightforward.
No immediate concerns.

---

202-232: Delete handler in a consistent pattern.
Adopting the same approach for resolving namespace and calling service is good.

---

234-255: Mapping expand parameters is logical.
Ensures expansions are correctly set, while filtering out invalid ones.

---

301-312: Profile mapping is cleanly delegated.
Reusing MapProfileToApi ensures consistent transformations.

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (1)

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

16-84: 🛠️ Refactor suggestion

Unified create/update flow in UpsertCustomerOverride.

The transaction-based approach properly checks for an existing override, then updates or creates accordingly. This helps avoid code duplication in separate methods. Consider verifying concurrency if multiple simultaneous upserts occur—some unique constraint or lock might be needed to prevent race conditions. Otherwise, this function is well-structured.

♻️ Duplicate comments (1)

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

287-290: ⚠️ Potential issue

Fix the nil pointer dereference risk.

There's a TODO comment indicating a need to validate for nil pointer dereference at line 289, but the code at line 287 directly accesses in.MergedProfile.ID without any nil check. This could cause a runtime panic if in.MergedProfile is nil.

Apply this fix:

-   res.BaseBillingProfileId = in.MergedProfile.ID
-
-   // TODO: validate for nil pointer deref!
+   if in.MergedProfile == nil {
+       return res, fmt.Errorf("merged profile is nil, cannot map customer override to API")
+   }
+   res.BaseBillingProfileId = in.MergedProfile.ID
+
+   // Nil check addressed

🧹 Nitpick comments (6)

openmeter/billing/customeroverride.go (4)

139-163: Validation covers all relevant fields.

This ensures Namespace and CustomerID are non-empty, and delegates further validations to Collection, Invoicing, and Payment configs. Consider clarifying whether ProfileID must also undergo validation or remain optional given the fallback logic in the service layer.

---

214-220: CustomerOverrideWithDetails struct helps unify override & extended fields.

The inline pointer and new fields (Customer, MergedProfile, etc.) support an enriched response object. Watch out for potential nil references if CustomerOverride is missing from earlier logic. Otherwise, this is well structured.

---

239-245: Expanded customer filter fields.

Introducing CustomerID, CustomerName, CustomerKey, CustomerPrimaryEmail, etc., improves filtering capabilities. However, note that the field is CustomerID []string here, whereas some adapter code references CustomerIDs. Consider aligning the naming for consistency.

---

258-263: Expanded fields for apps and customer.

Renaming the JSON tag to match the field name ("customer" instead of "customers") might reduce confusion. The doc says "the customer should be included in the response," but the JSON key is "customers". Consider renaming for clarity.

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

139-232: Enhanced list method with expanded data.

The single query to fetch all relevant customers is more efficient than one-by-one lookups. The default profile fallback logic (lines 205-207) prevents incomplete overrides. The final mapping with resolveCustomerOverrideWithDetails ensures consistent object structure.
Potential improvement: If very large sets of customer IDs are passed, consider chunked queries or indexing.

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

301-312: Consider adding error context for improved debugging.

While the error handling is functional, adding context to the error returned from h.MapProfileToApi would make debugging easier.

-   profile, err := h.MapProfileToApi(&in)
-   if err != nil {
-       return api.BillingCustomerProfile{}, err
-   }
+   profile, err := h.MapProfileToApi(&in)
+   if err != nil {
+       return api.BillingCustomerProfile{}, fmt.Errorf("failed to map profile to API: %w", err)
+   }

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0ddea14 and 72787bd.

📒 Files selected for processing (22)

• openmeter/billing/adapter/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (4 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (10)

• openmeter/billing/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/customeroverride.go
• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go

🔇 Additional comments (14)

openmeter/billing/adapter/customeroverride.go (2)

202-209: Looks solid.

This pagination mapping logic correctly unwraps the mapped *billing.CustomerOverride and handles possible mapping errors. No immediate issues identified.

---

214-215: Consistency with new approach.

Switching to Where(billingcustomeroverride.CustomerID(input.Customer.ID)) and Where(billingcustomeroverride.Namespace(input.Customer.Namespace)) aligns with the migrated DeleteCustomerOverrideInput struct. Validation for Customer.ID and Customer.Namespace is presumably handled upstream.

openmeter/billing/customeroverride.go (6)

12-12: New import inclusion is fine.

Importing github.com/samber/lo seems necessary for the new utilities (e.g., lo.Contains). No issues here.

---

128-137: New UpsertCustomerOverrideInput struct is well-defined.

This struct centralizes fields used in both create and update flows. If ProfileID is empty, the service layer attempts to fetch the default profile, which is consistent with the approach in the adapter. Good separation of concerns.

---

202-205: Use of type aliases for create/update eliminates duplication.

These aliases reduce code repetition, making it clearer that both creation and update share the same underlying input. No concerns here.

---

265-268: Default expand set.

CustomerOverrideExpandAll neatly sets all booleans to true, making it straightforward to request full expansions. No issues here.

---

270-296: Ordering logic is robust.

Defining valid constants and storing them in CustomerOverrideOrderByValues with a dedicated Validate() method is a clean approach to ensuring only supported values are used.

---

300-306: Refined typed results for listing overrides.

Using generics-based pagination.PagedResponse[CustomerOverrideWithDetails] and pagination.PagedResponse[CustomerOverrideWithCustomerID] clarifies the data flow and improves type safety. No immediate concerns.

openmeter/billing/service/customeroverride.go (2)

5-5: Minor import.

Adding "fmt" is consistent with changes that output formatted errors or messages. Nothing else to note here.

---

86-130: GetCustomerOverride with fallback to default profile is logical.

• Good validation step at lines 89-94.
• Returning a dedicated NotFoundError if no override is found is appropriate.
• The fallback to fetch the default profile at lines 115-122 is consistent with the upsert logic.

No major pitfalls identified.

openmeter/billing/httpdriver/customeroverride.go (4)

22-26: Type updates align with expanded customer override details.

The type updates correctly reflect the enhanced response structure that now includes detailed information about customer overrides.

---

28-91: Comprehensive implementation of ListCustomerOverrides handler.

This handler implementation is well-structured with proper namespace resolution, parameter mapping, and error handling. The pagination and filtering parameters are correctly mapped from the API request to the service layer.

---

234-255: Well-implemented expand parameter mapping with validation.

The mapping function correctly handles nil inputs and validates expand parameters against allowed values, returning appropriate validation errors for invalid inputs.

---

257-286: Defensive implementation for customer override mapping.

The mapping function handles null values gracefully by checking if in.CustomerOverride and in.Customer are nil before attempting to access their fields, which prevents nil pointer dereferences.

[coderabbitai]

Actionable comments posted: 0

🔭 Outside diff range comments (2)

openmeter/billing/adapter/customeroverride.go (2)

25-29: ⚠️ Potential issue

Fix duplicate lines in create operation.

There's a duplication in the customer override creation chain where lines 25-27 are repeated, with line 26 being incomplete.

Apply this diff to fix the duplicated code:

-		_, err := tx.db.BillingCustomerOverride.Create().
-			SetNamespace(input.Namespace).
		_, err := tx.db.BillingCustomerOverride.Create().
			SetNamespace(input.Namespace).
			SetCustomerID(input.CustomerID).

---

281-329: ⚠️ Potential issue

Remove duplicate function implementation.

The GetCustomerOverrideReferencingProfile function appears to be defined twice with identical implementations at lines 281-303 and 307-329.

Apply this diff to remove the duplication:

-func (a *adapter) GetCustomerOverrideReferencingProfile(ctx context.Context, input billing.HasCustomerOverrideReferencingProfileAdapterInput) ([]customer.CustomerID, error) {
-	return entutils.TransactingRepo(ctx, a, func(ctx context.Context, tx *adapter) ([]customer.CustomerID, error) {
-		dbCustomerOverrides, err := tx.db.BillingCustomerOverride.Query().
-			Where(billingcustomeroverride.Namespace(input.Namespace)).
-			Where(billingcustomeroverride.BillingProfileID(input.ID)).
-			Where(billingcustomeroverride.DeletedAtIsNil()).
-			Select(billingcustomeroverride.FieldCustomerID).
-			All(ctx)
-		if err != nil {
-			return nil, err
-		}
-
-		var customerIDs []customer.CustomerID
-		for _, dbCustomerOverride := range dbCustomerOverrides {
-			customerIDs = append(customerIDs, customer.CustomerID{
-				Namespace: input.Namespace,
-				ID:        dbCustomerOverride.CustomerID,
-			})
-		}
-
-		return customerIDs, nil
-	})
-}
-

♻️ Duplicate comments (1)

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

287-289: ⚠️ Potential issue

Guard against potential nil pointer dereference.

Accessing in.MergedProfile.ID can panic if in.MergedProfile is nil. The TODO remark indicates awareness, but it remains unaddressed. Consider adding a null check before referencing in.MergedProfile.ID.

 if in.MergedProfile == nil {
     return res, fmt.Errorf("merged profile is nil, cannot map customer override to API")
 }
 res.BaseBillingProfileId = in.MergedProfile.ID

🧹 Nitpick comments (3)

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

16-84: UpsertCustomerOverride logic is robust but consider concurrency.

While the transaction helps ensure atomicity, concurrent requests updating the same override could cause a last-write-wins scenario. If strict concurrency control is required, consider an optimistic locking strategy or version field in the record to prevent silent overwrites.

openmeter/billing/customeroverride.go (1)

258-263: JSON tags for CustomerOverrideExpand appear correct, but double-check usage.

Line 262: Customer bool \json:"customers,omitempty"`might be better namedjson:"customer,omitempty"if referencing a single customer. Otherwise, ensure the parent code expectscustomers` in the JSON.

openmeter/billing/adapter/customeroverride.go (1)

199-202: Suggestion: Add explicit constant for default ordering.

For the default ordering case in the switch statement, consider using a constant rather than duplicating the logic from the default case.

-		case billing.CustomerOverrideOrderByCustomerCreatedAt:
-			query = query.Order(dbcustomer.ByCreatedAt(order...))
-		default:
-			query = query.Order(dbcustomer.ByID(order...))
+		case billing.CustomerOverrideOrderByCustomerCreatedAt:
+			query = query.Order(dbcustomer.ByCreatedAt(order...))
+		default:
+			// Default sort is by customer ID
+			query = query.Order(dbcustomer.ByID(order...))

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0ddea14 and 72787bd.

📒 Files selected for processing (22)

• openmeter/billing/adapter/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (4 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (9)

• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/service/customeroverride.go

🔇 Additional comments (34)

openmeter/billing/httpdriver/customeroverride.go (12)

4-18: No immediate issues with updated imports.

These newly added imports appear correct and align with the structured approach of this package. No security vulnerabilities or unused references are evident.

---

23-26: Type alias updates appear consistent.

Renaming these types to reference the new api.BillingProfileCustomerOverrideWithDetailsPaginatedResponse and api.ListBillingProfileCustomerOverridesParams is coherent with the expanded return structure and request parameters. This helps maintain clarity across layers.

---

28-91: Overall logic of ListCustomerOverrides is sound.

1. Validates and transforms request parameters.
2. Delegates to service for business logic.
3. Maps results to API struct with pagination.

No concurrency or security concerns stand out. Error handling is handled at each step and gracefully returns appropriate HTTP responses.

---

93-101: New param struct for GetCustomerOverride is clear.

Defining this struct in the HTTP layer with CustomerId and Expand fields helps maintain a clean separation of concerns and strongly typed parameters downstream.

---

103-144: Implementation of GetCustomerOverride aligns with established patterns.

1. Correctly retrieves the namespace and expand parameters.
2. Delegates to the service, then returns mapped results as JSON.
3. Error handling gracefully wraps lower-level errors.

No issues discovered.

---

146-153: Upsert request type definitions are consistent.

These new type aliases are straightforward and align with the new upsert approach. No issues with the JSON field naming.

---

155-191: UpsertCustomerOverride handler appears correct.

1. Follows the pattern of decoding JSON into UpsertCustomerOverrideRequest.
2. Delegates creation or update logic to the service.
3. Returns the final state to the client as a JSON response.

Error handling is consistent; no concurrency or security concerns are evident in this layer.

---

193-201: Param struct for DeleteCustomerOverride is well-defined.

The structured approach continues here, ensuring the customerId is collected correctly from the HTTP layer.

---

202-232: DeleteCustomerOverride handler logic is straightforward.

1. Resolves namespace and delegates to service.
2. Uses a No Content response upon success.

Implementation follows the established deletion pattern without notable issues.

---

234-255: Sensible mapping of expand parameters in mapCustomerOverrideExpandToEntity.

Switch-based expansion ensures invalid values are handled with a validation error. This is a clear, extensible pattern for controlling expansions.

---

257-299: Function mapCustomerOverrideToAPI is well-organized.

It translates the override and related fields into the API-friendly structure. The code is using explicit checks for the presence of in.CustomerOverride and expansions. No major logic issues identified.

---

301-312: mapCustomerProfileToAPI function is concise and correct.

This neatly converts an internal Profile into the expected API shape, delegating deeper field-level transformations to the MapProfileToApi utility. No issues discovered.

openmeter/billing/service/customeroverride.go (5)

86-130: GetCustomerOverride method handles lookups and merges effectively.

1. Validates inputs and resolves the override from the adapter/service.
2. Fetches a default profile if none is attached.
3. Delegates final combination to resolveCustomerOverrideWithDetails.

Error handling cleanly handles not found scenarios.

---

132-137: DeleteCustomerOverride method is consistent.

The transaction-based approach ensures any database operations are atomic. Code matches the pattern from other methods and returns a standard error on failure.

---

139-232: Pagination and expand logic in ListCustomerOverrides is well-organized.

1. Validates input and retrieves overrides in a paginated manner.
2. Uses a single query to fetch customer data for expansions.
3. Properly merges in a default profile if needed.
4. Finally maps each override to a more detailed struct with resolveCustomerOverrideWithDetails.

No correctness or performance concerns stand out.

---

234-271: resolveCustomerOverrideWithDetailsInput type is an effective aggregator.

By grouping the Override, default profile, expansion flags, and a potential customer cache in a single struct, the code remains organized. No potential pitfalls observed.

---

272-308: resolveCustomerOverrideWithDetails merges logic clearly.

1. Validates input.
2. Identifies whether an override is active or needs the default profile.
3. Expands customer data if requested, reusing a cache to prevent redundant lookups.
4. Adequately merges the override with the baseline profile.

Implementation is coherent with minimal duplication.

openmeter/billing/customeroverride.go (8)

12-12: Additional import for lo is fine.

This third-party library supports convenient pointer and slice operations. No issues observed regarding usage.

---

128-162: UpsertCustomerOverrideInput and Validate are well-defined.

1. Requires Namespace and CustomerID.
2. Ensures the override configs (Collection, Invoicing, Payment) pass their respective validations.

No logical flaws or missing fields seem apparent.

---

202-205: Renaming adapter inputs as type aliases is clear and reduces duplication.

Directly mapping UpdateCustomerOverrideAdapterInput to UpsertCustomerOverrideInput is a sensible approach to unify the creation and update logic.

---

214-221: CustomerOverrideWithDetails type accommodates merged profiles cleanly.

This approach clarifies the difference between the raw CustomerOverride and the final MergedProfile. Inline expansions for customer remain optional.

---

265-268: CustomerOverrideExpandAll is straightforward.

Setting both Apps and Customer to true helps quickly expand all relevant override data.

---

270-280: Introducing CustomerOverrideOrderBy improves sorting granularity.

By enumerating valid fields (customerId, customerName, etc.), you preserve type safety and provide domain-specific ordering. This makes future extension simpler.

---

282-299: Validation for CustomerOverrideOrderBy is well-defined.

Checking against CustomerOverrideOrderByValues ensures invalid order-bys are caught early. This is a good pattern for enumerated string fields.

---

300-310: Shift to PagedResponse[CustomerOverrideWithDetails] aligns with the new detail level.

Switching from CustomerOverrideWithMergedProfile to CustomerOverrideWithDetails clarifies the struct contents. No concerns.

openmeter/billing/adapter/customeroverride.go (9)

14-14: LGTM: Updated imports to support new filtering functionality.

The addition of imports for dbcustomer and sortx packages properly supports the enhanced filtering and ordering capabilities implemented in the ListCustomerOverrides method.

Also applies to: 18-18

---

23-23: Good design pattern: Improved input type separation.

The signature change from CreateCustomerOverrideInput to CreateCustomerOverrideAdapterInput establishes a clearer boundary between API/service layer inputs and adapter-specific inputs, promoting better separation of concerns.

---

78-79: LGTM: Enhanced update operation with tax config and simplified deletion.

Added tax configuration support and unconditionally clear the DeletedAt field to restore soft-deleted overrides, which simplifies the update logic and makes it more consistent.

---

104-152: LGTM: Improved GetCustomerOverride with transaction wrapper and default profile handling.

The refactoring of GetCustomerOverride to use a transaction wrapper and simplified error handling is excellent. The addition of logic to fetch the default billing profile when none is explicitly set is a robust enhancement that ensures customer overrides always have a billing profile.

---

156-246: LGTM: Enhanced ListCustomerOverrides with customer-based filtering.

The major refactoring provides comprehensive filtering capabilities based on customer fields (ID, name, key, email) and improved ordering options. Querying the Customer table directly is a smart approach to enable these advanced filters.

The pagination implementation and mapping to domain models is well-structured and properly handles error cases.

---

250-278: LGTM: Improved DeleteCustomerOverride with transaction support.

Using TransactingRepoWithNoValue for the delete operation ensures consistency and proper error handling within a transaction context. The soft delete approach by setting DeletedAt is preferable to hard deletes for maintaining data history.

---

156-158: Great documentation: Clear comment about customer field filtering.

The comment explaining why the customer DB parts are used for filtering is valuable for future maintainers to understand the design decision.

---

167-182: LGTM: Well-structured customer field filters.

The implementation of customer field filters with individual checks for each field type (ID, name, key, email) is clean, organized, and easy to follow. The containsFold functions for name and email allow for case-insensitive partial matching, which is excellent for user experience.

---

223-245: LGTM: Proper handling of customers without overrides.

The mapping function correctly handles the case where a customer doesn't have a billing override by returning a CustomerOverrideWithCustomerID with just the customer ID information, which is a robust approach.

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (1)

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

116-135: 💡 Verification agent

🧩 Analysis chain

Ensure consistency with the newly introduced return type.
This function introduces a return type of *billing.CustomerOverrideWithMergedProfile. Verify that other callers and related tests are properly updated to handle the pointer type rather than the previously used billing.CustomerOverrideWithDetails.

Would you like to run a quick repository-wide grep to confirm references to the old return type?

---

🏁 Script executed:

#!/bin/bash
rg -A 5 "CustomerOverrideWithDetails" 

Length of output: 26109

---

Attention: Inconsistent Return Type Usage Detected

It appears that while this function now returns a pointer to billing.CustomerOverrideWithMergedProfile, significant portions of the codebase—such as tests in test/billing/customeroverride_test.go, HTTP driver references in openmeter/billing/httpdriver/customeroverride.go, and interface definitions in openmeter/billing/service.go—are still using the older billing.CustomerOverrideWithDetails type. Please ensure that all callers, tests, OpenAPI specs, and generated client code are updated to handle the new pointer type.

• Update test cases (e.g., in test/billing/customeroverride_test.go) to expect *billing.CustomerOverrideWithMergedProfile.
• Revise service interface definitions and HTTP driver mappings to align with the new return type.
• Validate that all documentation and API contracts reflect this change.

🧹 Nitpick comments (4)

openmeter/billing/service/customeroverride.go (3)

163-168: Validate consistent error handling for missing overrides.
Here, you’re returning NotFoundError when existingOverride is nil. Ensure that higher-level calls handle this error gracefully and provide user-friendly feedback in consumers of this service.

---

242-249: Check handling of non-existent customers.
If customersByID lacks the entry, you return a NotFoundError. This is correct, but ensure that the upstream caller logs or surfaces this properly to avoid silent failures.

---

278-278: Avoid overshadowing the imported package name.
The local variable customer shadows the customer package import, which may cause confusion. Consider renaming the variable to cust or custEntity for clarity.

- customer, err := s.customerService.GetCustomer(ctx, customer.GetCustomerInput(input.Customer))
+ custEntity, err := s.customerService.GetCustomer(ctx, customer.GetCustomerInput(input.Customer))

openmeter/billing/customeroverride.go (1)

265-268: CustomerOverrideExpandAll is convenient.
Enabling both Apps and Customer by default is a handy approach. Just ensure you truly want all expansions in typical calls.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0ddea14 and 72787bd.

📒 Files selected for processing (22)

• openmeter/billing/adapter/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (4 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/adapter/customeroverride.go (3 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• openmeter/billing/adapter/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/httpdriver/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (11)

• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/httpdriver/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/adapter/customeroverride.go
• openmeter/billing/adapter/customeroverride.go

🔇 Additional comments (25)

openmeter/billing/service/customeroverride.go (5)

153-154: No issues found.
These lines correctly use input.Customer.Namespace and input.Customer.ID, aligning with the pattern established elsewhere in the file.

---

222-225: Efficient check for needing default profile.
Using lo.Find is clean here, but be mindful of performance if res.Items grows significantly. Otherwise, this approach looks solid.

---

236-239: Good usage of pagination mapping.
Returning a custom type in the mapping function provides a clearer structure. No issues found.

---

255-264: Fallback to default profile is clear.
The usage of lo.CoalesceOrEmpty followed by an error if the profile is still nil is straightforward. This is good defensive coding.

---

173-176:

❓ Verification inconclusive

Confirm whether soft-deleted overrides should return a different status.
When DeletedAt is non-nil, the code throws billing.ErrCustomerOverrideAlreadyDeleted. If the desired behavior is to allow re-creation or upsert, consider adjusting the logic accordingly.

Do you want a script to locate references to billing.ErrCustomerOverrideAlreadyDeleted across the codebase and confirm they’re aligned with this usage?

---

🏁 Script executed:

#!/bin/bash
rg -A 3 "ErrCustomerOverrideAlreadyDeleted"

Length of output: 735

---

Action Required: Clarify Soft-Deleted Override Handling

The current implementation in openmeter/billing/service/customeroverride.go returns billing.ErrCustomerOverrideAlreadyDeleted when an override’s DeletedAt is non-nil. A repository-wide search confirms that this error is consistently used (as defined in openmeter/billing/errors.go) for soft-deleted overrides. Could you please confirm whether soft-deleted overrides should maintain this behavior or if they need to allow re-creation/upsert by returning a different status? If the latter is desired, the logic here (and related checks) may need to be revised accordingly.

openmeter/billing/adapter/customeroverride.go (3)

202-209: LGTM on simplified return structure.
Switching to return a plain billing.CustomerOverride is cleaner. Confirm that any code expecting a pointer can handle this updated return type, if needed.

---

214-215: Transitions to input.Customer.ID and input.Customer.Namespace.
This matches the new struct approach for DeleteCustomerOverrideInput. The usage is consistent with the rest of the code.

---

222-225: Check for correct 404 on zero rows updated.
In lines 222-225, you handle the scenario where rowsAffected == 0. This is appropriate to ensure a “not found” error for an already-deleted or non-existent override.

openmeter/billing/customeroverride.go (7)

12-12: Confirmed addition of github.com/samber/lo import.
No concerns here as it’s used across your new logic.

---

128-137: Creating UpsertCustomerOverrideInput is a solid approach.
Consolidating create and update logic reduces duplication. The struct fields look coherent.

---

202-205: Aliases reduce code duplication.
UpdateCustomerOverrideAdapterInput = UpsertCustomerOverrideInput and CreateCustomerOverrideAdapterInput = UpdateCustomerOverrideAdapterInput help unify the logic.

---

258-263: [Duplicate comment] Fix JSON tag mismatch for “customer”.
The field name is Customer but the JSON tag is customers. Consider using json:"customer,omitempty" to maintain consistency.

---

270-296: Ordering constants and validation look good.
Your approach ensures an error if an unknown CustomerOverrideOrderBy value is provided. This is a solid pattern.

---

300-308: Switching to typed paged responses.
Returning typed results with pagination.PagedResponse[CustomerOverrideWithDetails] or [CustomerOverrideWithCustomerID] clarifies usage at call sites. This is a best practice.

---

139-163:

✅ Verification successful

Possible bracket mismatch around line 143.
Verify that the extra brace is correct or remove it if unintentional. This might cause a syntax error.

---

🏁 Script executed:

#!/bin/bash
# Check for extra braces in customeroverride.go
ast-grep --pattern $'func $_($_) {\n$$$}' 

Length of output: 73824

---

Bracket Mismatch Not Detected
After re-verifying the code in openmeter/billing/customeroverride.go (lines 139–163) and rerunning the AST search, no extra or unmatched braces were found in the Validate function. The function’s braces are balanced and correct.

openmeter/billing/httpdriver/customeroverride.go (10)

4-19: Imports look consistent and necessary.
No issues with these new imports; they appear correctly scoped to the functionality below.

---

23-26: Type definitions align well with the updated API.
These aliases correctly match the new paginated response and request structures.

---

28-91: Well-structured handler for listing overrides.
Input parameters and error handling look solid. The approach to expansions and pagination is appropriate for this read operation.

---

93-144: Straightforward data retrieval with expansion support.
The “GetCustomerOverride” flow is properly validated, and the mapping to/from model is clean.

---

146-153: New request types are clearly declarative.
They capture the upsert logic inputs in a consistent manner.

---

155-191: Potential concurrency conflict in Upsert.
This upsert can overwrite changes if multiple requests happen simultaneously. If concurrency is a concern, consider employing a version or ETag check to avoid lost updates.

---

193-232: Deletion handler is consistent with REST standards.
Returning a “204 No Content” on successful deletion is appropriate.

---

234-255: Expansion parsing is implemented correctly.
The logic properly interprets the requested expands and stops if an invalid option is supplied.

---

257-299: Address the TODO for nil pointer dereference.
Accessing in.MergedProfile.ID may panic if in.MergedProfile is nil. A quick safety check ensures stability.

Apply a fix like this:

 if in.MergedProfile == nil {
+    return res, fmt.Errorf("merged profile is nil, cannot map customer override to API")
 }
 res.BaseBillingProfileId = in.MergedProfile.ID

---

301-312: Customer profile mapping appears correct.
No issues found; the function cleanly converts the internal profile structure for external use.

[coderabbitai]

Actionable comments posted: 0

🔭 Outside diff range comments (1)

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

27-29: ⚠️ Potential issue

Duplicate assignment of Namespace detected.
Lines 28 and 29 both assign Namespace: input.Namespace, which is likely a copy/paste error and may cause confusion.

Apply this diff to remove the duplicate assignment:

  existingOverride, err := s.adapter.GetCustomerOverride(ctx, billing.GetCustomerOverrideAdapterInput{
-	Customer: customer.CustomerID{
-		Namespace: input.Namespace,
-		Namespace: input.Namespace,
-		ID:        input.CustomerID,
+	Customer: customer.CustomerID{
+		Namespace: input.Namespace,
+		ID:        input.CustomerID,
 	},

🧹 Nitpick comments (1)

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

57-69: Creation or update logic is streamlined but watch for repeated declarations.
You are redeclaring var upsertedOverride *billing.CustomerOverride separately (line 39 appears to declare it as well). Consider collapsing duplicate variable declarations to maintain clarity.

- var upsertedOverride *billing.CustomerOverride
  ...

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 72787bd and e730ab0.

📒 Files selected for processing (13)

• openmeter/billing/customeroverride.go (2 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (3 hunks)
• openmeter/billing/service/customeroverride.go (11 hunks)
• openmeter/billing/customeroverride.go (6 hunks)
• openmeter/billing/service/customeroverride.go (8 hunks)
• openmeter/billing/service/customeroverride.go (2 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)
• openmeter/billing/customeroverride.go (1 hunks)
• openmeter/billing/service/customeroverride.go (1 hunks)

✅ Files skipped from review due to trivial changes (2)

• openmeter/billing/customeroverride.go
• openmeter/billing/service/customeroverride.go

🚧 Files skipped from review as they are similar to previous changes (5)

• openmeter/billing/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/customeroverride.go
• openmeter/billing/service/customeroverride.go
• openmeter/billing/service/customeroverride.go

⏰ Context from checks skipped due to timeout of 90000ms (2)

• GitHub Check: CI
• GitHub Check: Analyze (go)

🔇 Additional comments (9)

openmeter/billing/service/customeroverride.go (5)

5-5: No issues with newly added import.
The import of "fmt" appears to be legitimate and necessary for error handling.

---

16-20: UpsertCustomerOverride signature looks good.
The consolidation into an UpsertCustomerOverride method effectively centralizes creation/update logic. Input validation and the approach of returning a default struct on errors appear consistent with the rest of the codebase.

---

234-242: resolveCustomerOverrideWithDetailsInput structure is well-defined.
This struct neatly packages the needed references to override, default profile, expand flags, and a customer cache. The clear decomposition promotes maintainability.

---

309-367: Profile merging logic is comprehensive.
The resolveProfileWorkflow flows logically:

• If override is nil or deleted, use the default profile.
• Validate the merged profile once patched.

No apparent logic errors.

---

115-122:

❓ Verification inconclusive

Validate default profile assignment logic.
When the override has no profile, you're assigning the default one. Ensure that scenarios where no default profile is configured are handled thoroughly; currently, a NotFoundError is returned, which is correct. Just verify all calling code expects and manages that error appropriately.

---

🏁 Script executed:

#!/bin/bash
# Use AST or textual grep to find all calls to UpsertCustomerOverride to ensure they handle NotFoundError properly.
rg -A10 "UpsertCustomerOverride" 

Length of output: 21889

---

Default Profile Assignment Logic – Verification Needed

Please verify that our default profile assignment works properly in both scenarios:

• When an override lacks a profile: Confirm that if a default billing profile is configured, it’s correctly applied; otherwise, the function appropriately returns a billing.NotFoundError.
• Error propagation: Ensure that all upstream callers (HTTP handlers in openmeter/billing/httpdriver/customeroverride.go and test cases in test/billing/*) correctly expect and handle the NotFoundError when no default profile is configured.

The grep results indicate that most test cases are asserting no error (e.g., via require.NoError), so please double-check and, if needed, add explicit tests that validate the behavior when the default profile is absent.

openmeter/billing/customeroverride.go (4)

128-137: UpsertCustomerOverrideInput is a clean consolidation.
Merging creation and update fields into a single input struct simplifies the code and reduces duplication. Just ensure partial updates are intentionally unsupported or clarified.

---

262-263: Inconsistent JSON tag naming.
A previous review comment flagged that "customers,omitempty" might be better as "customer,omitempty". Using plural can create confusion given that the field is named Customer.

- Customer bool `json:"customers,omitempty"`
+ Customer bool `json:"customer,omitempty"`

---

270-280: CustomerOverrideOrderBy enums look correct.
The set of constants and validation for CustomerOverrideOrderBy is robust and covers expected fields like customerId, customerName, etc.

---

300-306: Good use of pagination generics.
Defining ListCustomerOverridesResult as a PagedResponse[CustomerOverrideWithDetails], plus a struct for CustomerOverrideWithCustomerID, keeps the listing logic organized.