Fix PayPal double taxation and webhook payment recording issues #1749
Conversation
WalkthroughValidate numeric subtotal/tax from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
arifulhoque7
added
needs: testing
needs: dev review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
Lib/Gateway/Paypal.php (1)
1-1: Fix trailing whitespace to satisfy PHPCSThe inspections pipeline reports:
1-1: Whitespace found at end of line (phpcs).
Please remove trailing whitespace on line 1 (and any others PHPCS flags). For example:
-<?php +<?phpThis should clear the coding‑standards job.
🧹 Nitpick comments (2)
Lib/Gateway/Paypal.php (2)
1545-1548: Avoid surfacing raw PayPal error payloads directly to the end userYou now capture error details when no
idis present:$error_details = isset( $body['details'] ) ? wp_json_encode( $body['details'] ) : wp_remote_retrieve_body( $response ); throw new \Exception( 'Invalid response from PayPal - no order ID. Error: ' . $error_details );Given that
prepare_to_sendwraps this inwp_die( $e->getMessage() );, this can expose raw API responses (including internal error structures) directly to visitors. That may be noisy for users and can leak implementation details.Consider:
- Logging the full
$error_details(to a debug log or a dedicated logger) instead.- Throwing a generic, localized message to the UI, e.g. “Failed to create PayPal order, please contact support,” with maybe a short error code.
Example:
- $error_details = isset( $body['details'] ) ? wp_json_encode( $body['details'] ) : wp_remote_retrieve_body( $response ); - throw new \Exception( 'Invalid response from PayPal - no order ID. Error: ' . $error_details ); + $error_details = isset( $body['details'] ) ? wp_json_encode( $body['details'] ) : wp_remote_retrieve_body( $response ); + error_log( 'WPUF PayPal order error: ' . $error_details ); + throw new \Exception( __( 'Failed to create PayPal order. Please contact site administrator.', 'wp-user-frontend' ) );
1465-1485: Trimcustom_idpayload in payment order to follow PayPal best practices; subscription custom_id is already optimalPayPal's
custom_idfield is limited to 255 characters and best practice is to store only a compact external identifier (e.g., your system's primary key) rather than large blobs or sensitive/PII data for reconciliation. Your current payload (211 chars) fits within the limit, but webhook handlers never readfirst_name,last_name, orcustom_id—user and post data are instead looked up viaget_user_by('id', user_id)andget_post(item_number). Removing these fields reduces unnecessary PII exposure and aligns with PayPal's guidance.Apply the suggested diff to line 1472 to keep only:
type,user_id,item_number,coupon_id,subtotal,tax.The subscription
custom_idat line 1387 is already correctly structured without PII and needs no changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Lib/Gateway/Paypal.php(5 hunks)
🧰 Additional context used
🪛 GitHub Actions: Inspections
Lib/Gateway/Paypal.php
[error] 1-1: Whitespace found at end of line (phpcs).
| [ | ||
| 'name' => isset( $data['custom']['post_title'] ) ? $data['custom']['post_title'] : $data['item_name'], | ||
| 'quantity' => '1', | ||
| 'unit_amount' => [ | ||
| 'currency_code' => $data['currency'], | ||
| 'value' => number_format( $data['subtotal'], 2, '.', '' ), | ||
| ], | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| $payment_data = [ |
There was a problem hiding this comment.
Guard against double taxation without disabling legitimate wpuf_payment_amount filters
The new guard:
$correct_billing_amount = $billing_amount;
$billing_amount = apply_filters( 'wpuf_payment_amount', $billing_amount, $post_id );
if ( $tax_amount > 0 && $billing_amount !== $correct_billing_amount ) {
$billing_amount = $correct_billing_amount;
}does prevent the Pro tax filter from re‑taxing an already‑taxed amount, but it also effectively disables all legitimate usages of wpuf_payment_amount whenever tax is non‑zero (discounts, surcharges, gateways wanting to adjust the final amount, etc.). That’s a potentially breaking change for integrators.
A safer approach is to:
- Keep the filtered result by default.
- Add a narrower heuristic (or a dedicated filter) to detect/override the specific “double‑tax” case.
- Or expose a new filter that allows Pro to supply the final trusted amount and simultaneously update
$data['subtotal']and$data['tax'].
For example:
- $correct_billing_amount = $billing_amount;
- $billing_amount = apply_filters( 'wpuf_payment_amount', $billing_amount, $post_id );
-
- if ( $tax_amount > 0 && $billing_amount !== $correct_billing_amount ) {
- $billing_amount = $correct_billing_amount;
- }
+ $correct_billing_amount = $billing_amount;
+ $filtered_amount = apply_filters( 'wpuf_payment_amount', $billing_amount, $post_id );
+
+ $billing_amount = (float) $filtered_amount;
+
+ $billing_amount = apply_filters(
+ 'wpuf_paypal_correct_billing_amount',
+ $billing_amount,
+ $correct_billing_amount,
+ $tax_amount,
+ $data
+ );Then Pro can hook wpuf_paypal_correct_billing_amount to undo only the known double‑tax scenario, without blocking other consumers of wpuf_payment_amount.
🤖 Prompt for AI Agents
Lib/Gateway/Paypal.php lines 1293-1304: current logic unconditionally reverts
any wpuf_payment_amount filter when tax is non‑zero, breaking legitimate filters
(discounts/surcharges/gateway adjustments); instead, keep the filtered
$billing_amount by default and add a narrower mechanism to detect and correct
only the true double‑tax case—introduce and apply a dedicated filter (e.g.
wpuf_paypal_correct_billing_amount) or a heuristic that compares the filtered
change to the known $tax_amount (or expected tax delta) and only then
revert/adjust; ensure when you override you also update $data['subtotal'] and
$data['tax'] accordingly so external integrators can opt in to the safe
correction without losing other wpuf_payment_amount behaviors.
Rubaiyat-E-Mohammad
added
bug
QA Approved
sapayth
left a comment
sapayth
left a comment
There was a problem hiding this comment.
recheck the values with caution. we already have amount, tax amount etc. calculate and send the proper data to PayPal by separating them in billing amount and tax amount as per PayPal documentation. also recheck if we are storing data in our db table properly
Lib/Gateway/Paypal.php
Outdated
| $correct_billing_amount = $billing_amount; | ||
|
|
||
| // Apply filters but ensure we use the correct amount if tax was already calculated | ||
| $billing_amount = apply_filters( 'wpuf_payment_amount', $billing_amount, $post_id ); |
There was a problem hiding this comment.
we need to check from where the $billing_amount is getting modified using wpuf_payment_amount filter. if we revert the amount, the filter hook looses it's purpose
sapayth
added
needs: author reply
Improves PayPal payment processing by validating and separating subtotal and tax calculations, ensuring coupons are applied before tax, and adding debug logging. Updates pay-per-post logic in the frontend form AJAX handler to support both new and legacy form settings for better compatibility.
|
done @sapayth vai |
arifulhoque7
added
needs: dev review
Eliminated error_log statements from the Paypal::prepare_to_send function to clean up unnecessary debug output and improve code clarity.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Lib/Gateway/Paypal.php (2)
1467-1512: Well-structured PayPal purchase unit with proper tax handling.The conditional inclusion of the
breakdownanditemsarray only whentax_amount > 0ensures PayPal receives the pre-calculated tax rather than computing it again, which addresses the double taxation issue.The
custom_idpayload comprehensively includes all necessary data for webhook processing.One operational note: ensure merchants verify their PayPal account settings don't have automatic tax calculation enabled, as that could still cause tax to be applied even when a breakdown is provided. This is more of a configuration concern than a code issue.
1547-1549: Improved error logging will help with debugging PayPal issues.Including the detailed error response from PayPal in the exception message is a valuable addition for troubleshooting API failures.
Minor optimization: you call
wp_remote_retrieve_body( $response )potentially twice (lines 1548 and 1549). Consider storing it in a variable once:if ( ! isset( $body['id'] ) ) { - // Log the full error response for debugging - $error_details = isset( $body['details'] ) ? wp_json_encode( $body['details'] ) : wp_remote_retrieve_body( $response ); + $response_body = wp_remote_retrieve_body( $response ); + $error_details = isset( $body['details'] ) ? wp_json_encode( $body['details'] ) : $response_body; throw new \Exception( 'Invalid response from PayPal - no order ID. Error: ' . $error_details ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Lib/Gateway/Paypal.php(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Lib/Gateway/Paypal.php (1)
includes/Traits/TaxableTrait.php (2)
wpuf_tax_enabled(23-31)wpuf_current_tax_rate(385-447)
🔇 Additional comments (3)
Lib/Gateway/Paypal.php (3)
295-314: Validation improvements look solid.The explicit validation of subtotal and tax being present and numeric addresses the previous review concern about silent coercion to zero. The exception messages are clear and will aid debugging.
One minor consideration: the exact string equality check (line 312) after
number_formatworks well for typical cases but has no tolerance for rounding variance. If future requirements involve complex discount calculations or PayPal-side adjustments, consider allowing a small epsilon (e.g., ±$0.01) to prevent legitimate payments from being rejected due to floating-point precision issues.
335-337: LGTM!Using the validated expected values from
custom_dataensures consistency between what was sent to PayPal and what's recorded in the webhook handler.
1251-1252: Cleaner data source for return URL parameters.Using
$data['type']and$data['item_number']instead of extracting from$_GETis more reliable and maintainable, as it ensures consistency throughout the payment flow.
| // Store subtotal before tax | ||
| $subtotal = $billing_amount; | ||
|
|
||
| // Handle coupon if present | ||
| // Handle coupon if present - apply to subtotal BEFORE tax (same as Stripe) | ||
| $coupon_id = ''; | ||
| if ( isset( $_POST['coupon_id'] ) && ! empty( $_POST['coupon_id'] ) && | ||
| isset( $_POST['_wpnonce'] ) && wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['_wpnonce'] ) ), 'wpuf_payment_coupon' ) && | ||
| is_numeric( sanitize_text_field( wp_unslash( $_POST['coupon_id'] ) ) ) ) { | ||
| is_numeric( sanitize_text_field( wp_unslash( $_POST['coupon_id'] ) ) ) && | ||
| function_exists( 'wpuf_pro' ) ) { | ||
| $coupon_id = absint( sanitize_text_field( wp_unslash( $_POST['coupon_id'] ) ) ); | ||
| $billing_amount = wpuf_pro()->coupon->discount( $billing_amount, $coupon_id, $data['item_number'] ); | ||
| $subtotal = wpuf_pro()->coupon->discount( $subtotal, $coupon_id, $data['item_number'] ); | ||
| } | ||
|
|
||
| // Calculate tax on the (possibly discounted) subtotal | ||
| // Note: We do NOT apply wpuf_payment_amount filter here because it adds tax | ||
| // via Pro's Tax class, and we calculate tax separately to maintain proper | ||
| // subtotal/tax breakdown for PayPal API. This is the same pattern used by Stripe. | ||
| if ( $this->wpuf_tax_enabled() ) { | ||
| $tax_rate = $this->wpuf_current_tax_rate(); | ||
| $tax_amount = $subtotal * ( $tax_rate / 100 ); | ||
| $billing_amount = $subtotal + $tax_amount; | ||
| } else { | ||
| $coupon_id = ''; | ||
| $billing_amount = $subtotal; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all add_filter calls on wpuf_payment_amount
echo "=== Looking for add_filter calls on wpuf_payment_amount ==="
rg -n "add_filter.*wpuf_payment_amount" --type php
echo -e "\n=== Looking for apply_filters calls on wpuf_payment_amount ==="
rg -n "apply_filters.*wpuf_payment_amount" --type php
echo -e "\n=== Looking for direct filter hook references ==="
rg -n "wpuf_payment_amount" --type php -B 2 -A 2Repository: weDevsOfficial/wp-user-frontend
Length of output: 206
Verify scope of wpuf_payment_amount filter usage before unconditionally removing it.
The current implementation skips the wpuf_payment_amount filter to prevent double taxation from Pro's tax addition. However, this removes a filter that may be used for legitimate purposes beyond tax handling—such as discounts, surcharges, gateway adjustments, or custom integrations.
A comprehensive review is needed to determine:
- All existing uses of
wpuf_payment_amountfilter (both internal and documented for third parties) - Whether Pro's tax class is the only source of double-taxation issues, or if there are other filter callbacks causing problems
- Whether a more surgical fix (e.g., a flag to bypass only the Pro tax filter) would preserve backward compatibility
If the filter is widely used or documented as an extension point, removing it entirely is a breaking change that should be approached cautiously.
🤖 Prompt for AI Agents
In Lib/Gateway/Paypal.php around lines 1279 to 1302, the code unconditionally
skips applying the wpuf_payment_amount filter to avoid double-taxation from
Pro's Tax class; instead, audit and preserve the filter while preventing only
the Pro tax double-apply: search the codebase (and docs) for all uses of
wpuf_payment_amount to catalog legitimate callbacks, identify the Pro tax
callback(s) (e.g., class/method attached), and change the implementation to
either (a) call apply_filters('wpuf_payment_amount', $amount, ...) but
remove/temporarily bypass only the Pro tax filter via remove_filter/add_filter
around that call, or (b) add a dedicated boolean flag/argument that downstream
filters can check to skip only tax-related adjustments (and document the new
flag), ensuring all other third‑party hooks still run to preserve backward
compatibility.
3 participants
Fix: PayPal Double Taxation & Webhook Payment Recording Issues Close issue
This branch resolves multiple PayPal-related bugs involving tax calculations, payment breakdown handling, and webhook validation.
Issues Fixed
1. Double Taxation in PayPal
wpuf_amount_with_taxfilter was re-applying tax on an already-taxed amount.2. Incorrect PayPal Amount Breakdown
itemsarray and properamount.breakdown.3. Webhook Payments Not Recorded
(subtotal + tax).Files Updated
wp-content/plugins/wp-user-frontend/Lib/Gateway/Paypal.phpcustom_dataduring webhook processingTesting Results
✔️ Correct PayPal checkout amount: $25.30
✔️ Webhook correctly records completed PayPal payments
✔️ Correct breakdown sent:
Summary by CodeRabbit
Bug Fixes
Improvements
Compatibility
✏️ Tip: You can customize this high-level summary in your review settings.