[18.0][ADD] l10n_us_sales_tax_engine: Hybrid US Sales Tax Engine#173
[18.0][ADD] l10n_us_sales_tax_engine: Hybrid US Sales Tax Engine#173crrodrigueztrujillo wants to merge 1 commit into
Conversation
|
@crrodrigueztrujillo please squash your commits into the first one. |
|
@crrodrigueztrujillo please prefix the PR name with the targeted version, like "[18.0] [ADD] ..." |
rrebollo
left a comment
There was a problem hiding this comment.
First of all, thank you for the effort and value of this contribution to the OCA ecosystem, and especially for the US localization space.
I tried to perform a thorough review, but with more than 5k added lines it becomes very difficult to properly assess all aspects of the implementation. With that in mind, I wanted to share a couple of broader considerations beyond implementation details:
-
Splitting the solution into multiple addons could help significantly from a review and maintainability perspective by reducing the volume and scope of each component. Whether this also brings functional benefits in terms of avoiding unnecessary installations is something I cannot properly judge from my side.
-
I would also encourage considering reuse of patterns and solutions that the community has already adopted for common problems, such as
connector,connector_importer,component, orqueue_job. From my own experience, I know these frameworks are not always well documented and require a certain technical background, so sometimes reimplementing things from scratch feels simpler and faster. However, over time, solutions that initially seemed sufficient can start growing organically until they become difficult to maintain or extend.
I would be interested in hearing your thoughts on these points.
Once again, thank you for the contribution.
| return specific | ||
| # Fallback to generic rate (no category) | ||
| return self.search( | ||
| domain + [("product_tax_category_id", "=", False)], |
There was a problem hiding this comment.
| domain + [("product_tax_category_id", "=", False)], | |
| domain, |
There was a problem hiding this comment.
Applied. Also updated the test fixtures to align with the simplified lookup logic.
|
What about california ?? CDFTA apis are best source ? |
28408e1 to
8614b75
Compare
Hybrid sales tax calculation engine for all 50 US states. - Local rate database (Florida DOR seed data, 67 counties) - External API fallback chain: ZipTax > API Ninjas > TaxJar - Response cache with configurable TTL (default 30 days) - Immutable audit log per calculation - Nexus management per company/state - Product fiscal categories: TANGIBLE, SERVICE, FOOD, MEDICINE, DIGITAL, SHIPPING, EXEMPT - Sale Order and Invoice integration with auto-calculation on confirm - Internal REST endpoints for external system integration - Provider-agnostic architecture
8614b75 to
6140ec9
Compare
|
Done! All commits squashed into one. |
|
Done! Updated to |
|
@usamahassan-rt Great point! California's CDTFA is one of the most complex tax jurisdictions in the US. The architecture is provider-agnostic — adding a CDTFA-specific provider only requires implementing a new class extending |
|
Thank you for the thorough review @rrebollo and for taking the time despite the volume of changes. Regarding splitting into multiple addons — we agree it's worth considering as the module grows and we'll evaluate it for a follow-up PR. On the OCA frameworks ( All other inline suggestions have been applied — thank you for the detailed feedback! |
Following @rrebollo recommendation, maybe we could have a sale_tax_engine_base and then create a sale_tax_provider_xxx for each one of them, this way we could allow more contributions to this idea. What do you think ? |
|
@jelenapoblet Thank you for the suggestion — we really like the idea of a We're open to restructuring the module this way before merging if that's the direction the OCA community prefers. Could you and @rrebollo confirm if this is a requirement for approval, or a recommendation for a future iteration? That will help us prioritize the refactor accordingly. |
|
@crrodrigueztrujillo, I don't have merge superpowers — I just made a review. Your contribution already shows a lot of work, and I'm not here to make you do even more. That said, in my humble opinion, the distributed architecture is much cleaner. |
|
I think we're alligned @rrebollo , this way we'll have a better architecture and more contributors will be able to participate. |
Summary
This PR adds
l10n_us_sales_tax_engine, a provider-agnostic hybrid sales taxcalculation engine for all 50 US states, developed by @BinhexTeam.
for zero-latency lookups
(priority-ordered, skips providers that have exhausted their monthly limit)
provider, address)
SHIPPING, EXEMPT
/api/v1/us_tax/calculate,/api/v1/us_tax/rate_lookup)the engine core
Test plan
source = cachesource = exempt_rule, tax = 0Screenshots
11 screenshots included in
static/description/showing settings, menu,nexus, calculation logs, providers and tax rates.
Contribution by Carlos R. Rodriguez — @BinhexTeam