feat(tax): add universal tax fallback feature #277
Conversation
- Add enable_universal_tax setting toggle - Add universal_tax_rate setting (0-100%) - Implement apply_universal_tax_rate filter - Update translations for new strings - Apply universal rate when no country-specific rates match
WalkthroughAdds universal fallback tax rate mechanism to the tax system. Introduces new filter hook and method to apply a universal tax rate when no country-specific rates exist. Extends tax settings with admin fields to enable and configure the universal tax rate globally. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
inc/tax/class-tax.php (1)
174-203: New universal tax settings are consistent with behavior; optional style fix for alignment warningThe two new settings fields look coherent with the runtime logic:
enable_universal_taxis a toggle gated onenable_taxes, matching the runtimewu_should_collect_taxes()+enable_universal_taxchecks.universal_tax_rate:
- Numeric 0–100 with a default of 10%, consistent with the fallback default in
apply_universal_tax_rate.- Properly requires both
enable_taxesandenable_universal_tax, so it only surfaces when relevant.- Copy and translation domain match the rest of the file.
The only remaining item is a non-blocking style warning from the code quality checks about double-arrow alignment on the
'enable_taxes'key at line 199. If you want to satisfy that sniff, you can adjust spacing, e.g.:- 'enable_taxes' => 1, - 'enable_universal_tax' => 1, + 'enable_taxes' => 1, + 'enable_universal_tax' => 1,Functionally everything here aligns with the new feature’s intent.
lang/ultimate-multisite.pot (1)
18475-18510: Universal tax strings are clear; consider tiny wording tweakThe new strings for the universal fallback tax feature (“Universal Tax”, “Enable Universal Tax”, “Universal Tax Rate (%)”, and their descriptions) are clear and aligned with the feature’s behavior.
One optional polish point for consistency/grammar between the two descriptions:
- Current:
- “Enable a fallback universal tax rate when no country-specific rates match.”
- “Tax rate applied when no country-specific rate matches.”
Consider updating the first to singular for consistency:
-Enable a fallback universal tax rate when no country-specific rates match. +Enable a fallback universal tax rate when no country-specific rate matches.This would need to be changed in the PHP source (
inc/tax/class-tax.php:179), then the POT regenerated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/tax/class-tax.php(3 hunks)lang/ultimate-multisite.pot(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
inc/tax/class-tax.php (2)
inc/functions/tax.php (1)
wu_should_collect_taxes(20-23)inc/functions/settings.php (2)
wu_get_setting(37-40)wu_register_settings_field(88-91)
🪛 GitHub Check: Code Quality Checks
inc/tax/class-tax.php
[failure] 124-124:
There should be a comma after the last array item in a multi-line array.
[warning] 105-105:
The method parameter $cart is never used
[warning] 105-105:
The method parameter $tax_category is never used
[warning] 105-105:
The method parameter $country is never used
[warning] 199-199:
Array double arrow not aligned correctly; expected 9 space(s) between "'enable_taxes'" and double arrow, but found 5.
🪛 PHPMD (2.15.0)
inc/tax/class-tax.php
105-105: Avoid unused parameters such as '$country'. (undefined)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$tax_category'. (undefined)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$cart'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: PHP 7.4
- GitHub Check: PHP 8.4
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (3)
inc/tax/class-tax.php (1)
35-35: Filter registration for universal tax fallback looks correctHooking
apply_universal_tax_rateintowu_cart_applicable_tax_rateswith priority10and4accepted args matches the described design and keeps the fallback logic centralized in the tax class.lang/ultimate-multisite.pot (2)
12-14: POT header date update is fine
POT-Creation-Datereflects the new extraction time and is consistent with a regenerated POT; no action needed.
3637-3638: Tax-related source reference additions look consistentThe extra references for existing strings like “Tax Rate”, “Taxes”, and “Tax Rates” pointing into
inc/tax/class-tax.phpcorrectly align these labels with the new tax logic and UI, without altering the translatable text. No further changes required here.Also applies to: 4129-4131, 4940-4943, 18515-18551
| /** | ||
| * Applies universal tax rate when no specific rates are found. | ||
| * | ||
| * @since 2.0.0 | ||
| * @param array $rates Tax rates. | ||
| * @param string $country Country code. | ||
| * @param string $tax_category Tax category. | ||
| * @param object $cart Cart object. | ||
| * @return array | ||
| */ | ||
| public function apply_universal_tax_rate(array $rates, string $country, string $tax_category, $cart): array { | ||
|
|
||
| if (empty($rates) && wu_should_collect_taxes() && wu_get_setting('enable_universal_tax', false)) { | ||
| $universal_rate = wu_get_setting('universal_tax_rate', 10); | ||
|
|
||
| $rates = [ | ||
| [ | ||
| 'id' => 'universal-' . uniqid(), | ||
| 'title' => __('Universal Tax', 'ultimate-multisite'), | ||
| 'country' => '*', | ||
| 'state' => '', | ||
| 'city' => '', | ||
| 'tax_type' => 'percentage', | ||
| 'tax_amount' => $universal_rate, | ||
| 'tax_rate' => $universal_rate, | ||
| 'priority' => 1, | ||
| 'compound' => false, | ||
| 'type' => 'regular', | ||
| 'rate' => $universal_rate, | ||
| ] | ||
| ]; | ||
| } | ||
|
|
||
| return $rates; | ||
| } |
There was a problem hiding this comment.
Universal tax fallback logic is sound; fix trailing comma lint + consider unused param warnings
Functionally this method does what the PR describes:
- Only applies when there are no existing rates, taxes should be collected, and universal tax is enabled.
- Pulls the rate from settings (default 10) and builds a rate array consistent with existing tax rate structures (id, title, country wildcard, percentage type, rate fields, priority, etc.).
- Returns the original
$ratesunchanged when the condition is not met.
Two minor points:
- Trailing comma in multiline array (blocks code quality check)
The last element of the inner array ('rate' => $universal_rate) is missing a trailing comma, which the code-quality check flags as a failure. You can fix it like this:
- 'rate' => $universal_rate,
- ]
+ 'rate' => $universal_rate,
+ ],- Unused parameters in the callback
$country,$tax_category, and$cartare intentionally unused but required by the filter signature. I’d keep them for future-proofing and hook compatibility; if PHPMD noise becomes an issue, you could add a@SuppressWarnings(PHPMD.UnusedFormalParameter)annotation on the method docblock rather than changing the signature.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Applies universal tax rate when no specific rates are found. | |
| * | |
| * @since 2.0.0 | |
| * @param array $rates Tax rates. | |
| * @param string $country Country code. | |
| * @param string $tax_category Tax category. | |
| * @param object $cart Cart object. | |
| * @return array | |
| */ | |
| public function apply_universal_tax_rate(array $rates, string $country, string $tax_category, $cart): array { | |
| if (empty($rates) && wu_should_collect_taxes() && wu_get_setting('enable_universal_tax', false)) { | |
| $universal_rate = wu_get_setting('universal_tax_rate', 10); | |
| $rates = [ | |
| [ | |
| 'id' => 'universal-' . uniqid(), | |
| 'title' => __('Universal Tax', 'ultimate-multisite'), | |
| 'country' => '*', | |
| 'state' => '', | |
| 'city' => '', | |
| 'tax_type' => 'percentage', | |
| 'tax_amount' => $universal_rate, | |
| 'tax_rate' => $universal_rate, | |
| 'priority' => 1, | |
| 'compound' => false, | |
| 'type' => 'regular', | |
| 'rate' => $universal_rate, | |
| ] | |
| ]; | |
| } | |
| return $rates; | |
| } | |
| /** | |
| * Applies universal tax rate when no specific rates are found. | |
| * | |
| * @since 2.0.0 | |
| * @param array $rates Tax rates. | |
| * @param string $country Country code. | |
| * @param string $tax_category Tax category. | |
| * @param object $cart Cart object. | |
| * @return array | |
| */ | |
| public function apply_universal_tax_rate(array $rates, string $country, string $tax_category, $cart): array { | |
| if (empty($rates) && wu_should_collect_taxes() && wu_get_setting('enable_universal_tax', false)) { | |
| $universal_rate = wu_get_setting('universal_tax_rate', 10); | |
| $rates = [ | |
| [ | |
| 'id' => 'universal-' . uniqid(), | |
| 'title' => __('Universal Tax', 'ultimate-multisite'), | |
| 'country' => '*', | |
| 'state' => '', | |
| 'city' => '', | |
| 'tax_type' => 'percentage', | |
| 'tax_amount' => $universal_rate, | |
| 'tax_rate' => $universal_rate, | |
| 'priority' => 1, | |
| 'compound' => false, | |
| 'type' => 'regular', | |
| 'rate' => $universal_rate, | |
| ], | |
| ]; | |
| } | |
| return $rates; | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 124-124:
There should be a comma after the last array item in a multi-line array.
[warning] 105-105:
The method parameter $cart is never used
[warning] 105-105:
The method parameter $tax_category is never used
[warning] 105-105:
The method parameter $country is never used
🪛 PHPMD (2.15.0)
105-105: Avoid unused parameters such as '$country'. (undefined)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$tax_category'. (undefined)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$cart'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In inc/tax/class-tax.php around lines 95 to 129, the trailing comma is missing
on the last element of the inner rate array and PHPMD may flag unused parameters
($country, $tax_category, $cart); add a trailing comma after the 'rate' =>
$universal_rate array entry to satisfy the linter, and keep the unused
parameters but add a docblock annotation like
@SuppressWarnings(PHPMD.UnusedFormalParameter) to the method docblock to silence
PHPMD warnings while preserving the filter signature.
superdav42
left a comment
superdav42
left a comment
There was a problem hiding this comment.
I think I'd rather implement this by adding a new option to the country drop down, "Apply to all countries" that could be chosen on the tax rates configuration page. It would probably be simpler to implement and wouldn't need a new setting field. Did you consider this approach?
|
My thinking was: since the user has to go to the tax settings to enable it first, they should also be able to enable universal tax rates there, instead of having to go to a separate page. |
I understand but I want it to be in tax rates configuration page. I don't want to add more settings on the settings page unless they are going to be used by most users. A universal tax seems like an advanced option that belongs on the tax rates page. I've never seen any other software use a "universal tax" so it seems like a foreign concept and would confuse many users. |
Makes sense! I'll edit it. |
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.