feat(billing): dual-write tax_code_id/tax_behavior on invoice line adapter paths#4049
Merged
borbelyr-kong merged 4 commits intomainfrom Apr 1, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughTax handling in billing adapters is expanded to persist and query tax code IDs and tax behavior alongside TaxConfig, with mapping logic refactored to use BackfillTaxConfig for reconstruction instead of direct conversion. Changes span invoice line persistence, split line groups, and query expansion across multiple adapter files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
borbelyr-kong
added
release-note/ignore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openmeter/billing/adapter/stdinvoicelines.go (1)
147-153:⚠️ Potential issue | 🟠 MajorConflict updates still leave the legacy
tax_configstale.These two upserts now refresh
tax_code_idandtax_behavior, but they still do not updatetax_configon conflict. SinceBackfillTaxConfigonly fills missing fields, an old JSON blob can keep the previous Stripe/behavior and mask clears or tax-code switches for top-level lines and schema-level-1 detailed lines. AddingUpdateTaxConfig()here keeps the dual-write path consistent.💡 Suggested fix
UpdateQuantity(). UpdateChildUniqueReferenceID(). UpdateCreditsApplied(). UpdateChargeID(). + UpdateTaxConfig(). UpdateTaxCodeID(). UpdateTaxBehavior(). Exec(ctx)UpdateQuantity(). UpdateChildUniqueReferenceID(). UpdateCreditsApplied(). + UpdateTaxConfig(). UpdateTaxCodeID(). UpdateTaxBehavior(). Exec(ctx)Also applies to: 347-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openmeter/billing/adapter/stdinvoicelines.go` around lines 147 - 153, The upsert conflict update chain is missing UpdateTaxConfig(), leaving legacy tax_config JSON stale; in the upsert chains that call UpdateQuantity().UpdateChildUniqueReferenceID().UpdateCreditsApplied().UpdateChargeID().UpdateTaxCodeID().UpdateTaxBehavior().Exec(ctx) add .UpdateTaxConfig() before .Exec(ctx) so tax_config is refreshed on conflict; make the identical change to the other equivalent upsert chain that mirrors these updates (the second block that currently omits UpdateTaxConfig()) so both dual-write paths stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openmeter/billing/adapter/gatheringlines.go`:
- Around line 245-247: The conflict-resolution update chain
(UpdateChildUniqueReferenceID, UpdateTaxCodeID, UpdateTaxBehavior) omits
updating the legacy tax_config JSON, causing mapGatheringInvoiceLineFromDB
(which calls BackfillTaxConfig) to read stale tax info; add UpdateTaxConfig()
into that chained update so the tax_config JSON column is overwritten on
conflict and stays consistent with tax_code_id/tax_behavior changes.
In `@openmeter/billing/adapter/invoicelinesplitgroup.go`:
- Around line 168-178: The mapper taxCodeFromSplitLineGroupEdge assumes the
TaxCode edge is loaded but some callers (CreateSplitLineGroup,
UpdateSplitLineGroup and GetSplitLineGroupHeaders) do not include WithTaxCode(),
leading to partial TaxConfig; fix by either reloading the entity with
WithTaxCode() before mapping in those callers or add a fallback inside
taxCodeFromSplitLineGroupEdge that uses the raw dbGroup.TaxCodeID (or similar
tax_code_id field) to resolve/map a TaxCode via taxcodeadapter when the edge is
missing; update both the occurrences around lines mentioned (including the block
at 224-228) so all code paths return a consistent TaxCode-derived result.
---
Outside diff comments:
In `@openmeter/billing/adapter/stdinvoicelines.go`:
- Around line 147-153: The upsert conflict update chain is missing
UpdateTaxConfig(), leaving legacy tax_config JSON stale; in the upsert chains
that call
UpdateQuantity().UpdateChildUniqueReferenceID().UpdateCreditsApplied().UpdateChargeID().UpdateTaxCodeID().UpdateTaxBehavior().Exec(ctx)
add .UpdateTaxConfig() before .Exec(ctx) so tax_config is refreshed on conflict;
make the identical change to the other equivalent upsert chain that mirrors
these updates (the second block that currently omits UpdateTaxConfig()) so both
dual-write paths stay consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f63b03ac-7488-481a-9407-914237879b81
📒 Files selected for processing (5)
openmeter/billing/adapter/gatheringinvoice.goopenmeter/billing/adapter/gatheringlines.goopenmeter/billing/adapter/invoicelinesplitgroup.goopenmeter/billing/adapter/stdinvoicelinemapper.goopenmeter/billing/adapter/stdinvoicelines.go
Comment on lines
+168
to
+178
| func taxCodeFromSplitLineGroupEdge(dbGroup *db.BillingInvoiceSplitLineGroup) *taxcode.TaxCode { | ||
| tc, err := dbGroup.Edges.TaxCodeOrErr() | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| mapped, err := taxcodeadapter.MapTaxCodeFromEntity(tc) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| return &mapped | ||
| } |
Contributor
There was a problem hiding this comment.
Some split-line-group reads can still return a partial TaxConfig.
This mapper now relies on taxCodeFromSplitLineGroupEdge, but not every caller loads that edge. CreateSplitLineGroup and UpdateSplitLineGroup still map the row straight from Save(), and GetSplitLineGroupHeaders at Line 397 still queries without WithTaxCode(), so rows that rely on the new columns can come back without TaxCodeID or derived Stripe data. A refetch with WithTaxCode() or a fallback from the raw tax_code_id field would keep these responses consistent.
Also applies to: 224-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openmeter/billing/adapter/invoicelinesplitgroup.go` around lines 168 - 178,
The mapper taxCodeFromSplitLineGroupEdge assumes the TaxCode edge is loaded but
some callers (CreateSplitLineGroup, UpdateSplitLineGroup and
GetSplitLineGroupHeaders) do not include WithTaxCode(), leading to partial
TaxConfig; fix by either reloading the entity with WithTaxCode() before mapping
in those callers or add a fallback inside taxCodeFromSplitLineGroupEdge that
uses the raw dbGroup.TaxCodeID (or similar tax_code_id field) to resolve/map a
TaxCode via taxcodeadapter when the edge is missing; update both the occurrences
around lines mentioned (including the block at 224-228) so all code paths return
a consistent TaxCode-derived result.
turip
approved these changes
Apr 1, 2026
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Tests