Skip to content

[PM-37599] refactor: Remove PM37597 always-enable-Stripe-automatic-tax feature flag - #7989

Merged
amorask-bitwarden merged 5 commits into
mainfrom
billing/PM-37599/remove-ff-always-enable-stripe-automatic-tax
Jul 24, 2026
Merged

[PM-37599] refactor: Remove PM37597 always-enable-Stripe-automatic-tax feature flag#7989
amorask-bitwarden merged 5 commits into
mainfrom
billing/PM-37599/remove-ff-always-enable-stripe-automatic-tax

Conversation

@amorask-bitwarden

@amorask-bitwarden amorask-bitwarden commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

Jira: PM-37599 — final story of epic PM-36315 "Simplify tax logic". Predecessors shipped in 2026.6.0: PM-37597 (#7662), PM-37598 (#7677), PM-37820 defect (#7719).

📔 Objective

Removes the PM37597_AlwaysEnableStripeAutomaticTax feature flag (LaunchDarkly key pm-37597-always-enable-stripe-automatic-tax) and deletes the now-dead flag-OFF tax logic, making the flag-ON behavior unconditional. The flag has been at 100% in production since 2026.6.0, so this is a pure code-debt removal with no end-user behavior change.

What this locks in

  • Stripe is the source of truth for taxability; new customers default to tax_exempt = "none" (taxable) and automatic_tax stays enabled.
  • Engineering no longer programmatically writes tax_exempt, so a CS-set exemption in Stripe is never overwritten.
  • The tax-ID banner is gated on Stripe's view of taxability; US customers never see it (preserves PM-37820).

Changes

  • Delete the 11 flag-OFF tax_exempt write branches across the org/provider/premium/payment commands, UpcomingInvoiceHandler, and ProviderBillingService (keeping the already-unconditional automatic_tax writes).
  • Collapse the tax-ID banner gating in GetOrganizationWarningsQuery and GetProviderWarningsQuery to the flag-ON conditions.
  • Delete TaxHelpers; relocate the {US, CH} postal-code check onto OrganizationCreateRequestModel as RequiresBillingPostalCode (runtime behavior unchanged).
  • Inline and remove GetCustomerWhileEnsuringCorrectTaxExemptionAsync (its reconciliation is dead once the flag is always-true).
  • Remove the now-unused IFeatureService dependency from 7 classes and delete the flag constant.

No contract or data-model change — the warnings response shape and banner conditions are exactly what production already serves. Server-only, V±2 safe.

Post-merge ops (outside this repo): archive the LaunchDarkly flag pm-37597-always-enable-stripe-automatic-tax.

…x feature flag

The pm-37597-always-enable-stripe-automatic-tax flag has been at 100% in
production since 2026.6.0, so its flag-OFF path is dead code. Remove the flag
and delete the legacy branches that programmatically wrote Stripe tax_exempt,
making the flag-ON behavior unconditional: Stripe is the source of truth for
taxability (customers default to tax_exempt = "none") and the tax-ID banner is
gated on Stripe's view rather than a hardcoded country list.

No end-user behavior change. Relocates the postal-code country check off the
now-deleted TaxHelpers onto OrganizationCreateRequestModel.

Final story of epic PM-36315.
@amorask-bitwarden amorask-bitwarden added the ai-review Request a Claude code review label Jul 15, 2026
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This is a pure tech-debt removal of the PM37597_AlwaysEnableStripeAutomaticTax feature flag, which has been at 100% since 2026.6.0. It deletes the dead flag-OFF tax_exempt write branches, removes TaxHelpers, collapses the tax-ID banner gating to the flag-ON conditions, and drops the now-unused IFeatureService dependency from the affected classes. Reviewed all 11 flag-OFF removals, the inlining of GetCustomerWhileEnsuringCorrectTaxExemptionAsync into a direct GetCustomerOrThrow, and the relocation of the {US, CH} postal-code check onto OrganizationCreateRequestModel — all preserve the flag-ON runtime behavior. No security, correctness, or breaking-change concerns were found.

Code Review Details

No findings.

Verification notes:

  • No remaining references to PM37597, TaxHelpers, GetCustomerWhileEnsuringCorrectTaxExemptionAsync, or Bit.Core.Billing.Tax.Utilities anywhere in the codebase.
  • IFeatureService correctly retained in GetOrganizationWarningsQuery (PM35215) and UpdateBillingAddressCommand (PM32645/PM32581), where it still gates other flags.
  • RequiresBillingPostalCode matches the deleted TaxHelpers.IsDirectTaxCountry logic exactly (same {US, CH} set and null/empty guard).
  • The existing reviewer thread on OrganizationCreateRequestModelTests.cs (split postal-code theory into true/false tests) is already addressed in the current test file.

@amorask-bitwarden amorask-bitwarden added the t:tech-debt Change Type - Tech debt label Jul 15, 2026
@amorask-bitwarden
amorask-bitwarden marked this pull request as ready for review July 15, 2026 18:59
@amorask-bitwarden
amorask-bitwarden requested review from a team as code owners July 15, 2026 18:59
connerbw
connerbw previously approved these changes Jul 15, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.22%. Comparing base (3e79593) to head (0f779db).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ling/Providers/Queries/GetProviderWarningsQuery.cs 75.00% 0 Missing and 1 partial ⚠️
...anizations/Queries/GetOrganizationWarningsQuery.cs 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7989      +/-   ##
==========================================
- Coverage   62.23%   62.22%   -0.01%     
==========================================
  Files        2287     2286       -1     
  Lines       99663    99507     -156     
  Branches     9004     8969      -35     
==========================================
- Hits        62023    61917     -106     
+ Misses      35469    35430      -39     
+ Partials     2171     2160      -11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amorask-bitwarden
amorask-bitwarden merged commit a44fba5 into main Jul 24, 2026
52 of 53 checks passed
@amorask-bitwarden
amorask-bitwarden deleted the billing/PM-37599/remove-ff-always-enable-stripe-automatic-tax branch July 24, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants