feat(billing): manage product prices through UpdateProduct

[rohilsurana]

What

Teach UpdateProduct to manage a product's prices. When the request carries a non-empty list of prices, product.Update converges the product's prices to that list. An empty list leaves prices untouched.

Builds on #1811 (merged)

The consumer-side hardening in #1811 (ignore inactive prices in checkout, plan matching, and invoicing) is merged to main, so this PR now stands alone and its diff is only the UpdateProduct changes.

How it behaves

The price list is authoritative when it is not empty. A price is identified by its name within the product.

• The list is validated before the product is touched, so an invalid price fails the whole update instead of half-applying the product's other changes.
• Names must be present and unique in the list.
• A name the product does not have yet is created.
• A name that already exists must keep its immutable fields (amount, currency, interval, billing scheme, usage type). A change to any of them is rejected with a clear error that tells you to add a new price under a new name, since provider prices cannot be changed in place.
• A retired price named again in the list is reactivated, not recreated.
• An active price the list no longer names is retired: marked inactive in the provider and the database, never deleted.
• Adds and reactivations run before retires, so the product always has the new price before an old one goes inactive.
• Applying the same list again makes no further changes.

Changes

• product.Update reads and validates the desired prices up front (validateDesiredPrices), then applies convergence (applyPriceConvergence); reactivatePrice and deactivatePrice share one setPriceActive helper that skips the provider call when there is no provider id.
• The price repository UpdateByID persists the state column, so a price can be retired or reactivated.
• The UpdateProduct handler reads the full price fields and reports a rejected price as InvalidArgument.
• AddPlan no longer triggers price convergence when it only links a plan to a product.

Tests

Update price convergence is covered: add a price, retire an omitted active price, reactivate a re-listed retired price, reject an in-place amount or currency change, reject a duplicate or empty name, an empty list leaves prices untouched, and applying an unchanged list makes no changes. Validation errors are asserted to happen before any product mutation.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	Jul 29, 2026 6:48am

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Product updates now validate and synchronize requested prices, including creating, reactivating, or deactivating prices as needed.
  • Price names are matched consistently despite differences in capitalization or surrounding whitespace.
• Bug Fixes
  • Prevented unintended changes to existing price details and database state.
  • Invalid price updates now return a clear invalid-argument error.
  • Linking a plan no longer unexpectedly changes its prices.

Walkthrough

Product updates now validate and converge desired prices across Stripe and the repository, while plan linking skips convergence. The billing API preserves optional metadata and maps invalid details to invalid-argument errors. Sparse price updates no longer clear stored state.

Changes

Product price convergence

Layer / File(s)	Summary
Price validation and convergence billing/product/service.go, billing/product/service_test.go	Service.Update validates canonical names and immutable fields before mutation, then creates, activates, or deactivates prices as needed. AddPlan skips convergence, and tests cover the convergence scenarios and operation ordering.
Billing API and price persistence internal/api/v1beta1connect/billing_product.go, internal/store/postgres/billing_price_repository.go	Product price parsing preserves mutable fields and optional metadata, invalid details return connect.CodeInvalidArgument, and sparse updates no longer overwrite stored state with an empty value.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• raystack/frontier#1811: Covers inactive-price filtering in billing consumers, complementing this PR’s price activation and deactivation behavior.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coveralls]

Coverage Report for CI Build 30427012558

Coverage increased (+0.1%) to 47.032%

Details

• Coverage increased (+0.1%) from the base build.
• Patch coverage: 35 uncovered changes across 3 files (116 of 151 lines covered, 76.82%).
• 39 coverage regressions across 2 files.

Uncovered Changes

File	Changed	Covered	%
billing/product/service.go	125	99	79.2%
internal/store/postgres/billing_price_repository.go	5	0	0.0%
internal/api/v1beta1connect/billing_product.go	21	17	80.95%

Coverage Regressions

39 previously-covered lines in 2 files lost coverage.

File	Lines Losing Coverage	Coverage
internal/bootstrap/schema/schema.go	35	18.7%
internal/bootstrap/generator.go	4	79.65%

---

Coverage Stats

Relevant Lines:	39199
Covered Lines:	18436
Line Coverage:	47.03%
Coverage Strength:	14.38 hits per line

---

💛 - Coveralls

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

billing/product/service.go (1)

385-401: 🩺 Stability & Availability | 🔵 Trivial

Provider and repo state can diverge on partial failure.

setPriceActive updates Stripe first and the repo second; if UpdateByID fails (or the process dies in between), the provider price is deactivated while the DB still reports it active, and the same applies mid-way through applyPriceConvergence. Consider a reconciliation job or logging the price ID and provider ID on failure so the drift is detectable.

billing/product/service_test.go (1)

569-586: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a case that pins the add-before-retire ordering.

applyPriceConvergence documents that creates/activations run before deactivations, but no test asserts it. mock.InOrder (or a Run hook recording call order) on the create and UpdateByID expectations would lock that invariant in. A whitespace-in-name case would also be worth adding alongside the normalization fix flagged in billing/product/service.go.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 53257abb-e1c9-4afe-b626-be07b3d2f867

📥 Commits

Reviewing files that changed from the base of the PR and between ab8b990 and ae9e1bc.

📒 Files selected for processing (11)

• billing/checkout/service.go
• billing/invoice/service.go
• billing/plan/plan.go
• billing/plan/plan_test.go
• billing/plan/service.go
• billing/product/product.go
• billing/product/service.go
• billing/product/service_test.go
• billing/subscription/service.go
• internal/api/v1beta1connect/billing_product.go
• internal/store/postgres/billing_price_repository.go

[rohilsurana]

Also addressed the two nitpicks:

• Add-before-deactivate ordering: added a test, "adds a price before it deactivates an omitted one", that records call order with Run hooks and asserts the create happens before the deactivate. The whitespace-name case is covered by the test noted on the normalization thread.
• Provider/repo drift in setPriceActive: this is the accepted converge-not-transact tradeoff, and the product service has no logger wired in today, so I have not added logging in this PR. Re-applying the same desired list self-corrects the case where the price stays omitted (it is deactivated again). The narrow mid-call-failure case where Stripe succeeds but the DB write fails and the price is still desired would not self-heal, and closing that fully needs a reconciler or a saga, which is out of scope here. Happy to file a follow-up if you'd like.

[coderabbitai]

🧹 Nitpick comments (2)

billing/product/service_test.go (1)

523-567: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the sentinel error, not just non-nil.

These rejection subtests pass for any error (including an unrelated mock/plumbing failure). Since internal/api/v1beta1connect/billing_product.go maps only product.ErrInvalidDetail to CodeInvalidArgument, asserting the sentinel here locks in that cross-layer contract.

♻️ Example for the amount-change subtest

-		if _, err := svc.Update(ctx, desired(changed)); err == nil {
-			t.Fatalf("Update() expected an error for an in-place amount change, got nil")
-		}
+		_, err := svc.Update(ctx, desired(changed))
+		if !errors.Is(err, product.ErrInvalidDetail) {
+			t.Fatalf("Update() error = %v, want %v", err, product.ErrInvalidDetail)
+		}

Same applies to the currency, duplicate-name, empty-name, and metered-aggregate subtests.

internal/api/v1beta1connect/billing_product.go (1)

165-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Map product.ErrProductNotFound to 404 in UpdateProduct.

BillingProductRepository.GetByID returns product.ErrProductNotFound on missing rows, so this branch can wrap it in connect.CodeInternal/500. Switch on the exact sentinel so a bad/existing product ID is returned as 404 rather than an internal service error.

♻️ Suggested extension

 		code := transformProductToPB(updatedProduct)
-		if errors.Is(err, product.ErrInvalidDetail) {
+		switch {
+		case errors.Is(err, product.ErrInvalidDetail):
 			code = connect.CodeInvalidArgument
+		case errors.Is(err, product.ErrProductNotFound):
+			code = connect.CodeNotFound
 		}

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 73463afb-bffa-4b4d-9d88-ce3404f4802d

📥 Commits

Reviewing files that changed from the base of the PR and between ae9e1bc and 892185c.

📒 Files selected for processing (4)

• billing/product/service.go
• billing/product/service_test.go
• internal/api/v1beta1connect/billing_product.go
• internal/store/postgres/billing_price_repository.go

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

• internal/store/postgres/billing_price_repository.go
• billing/product/service.go