Fix PayPal double taxation and webhook payment recording issues #1749
Conversation
WalkthroughValidate numeric 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 GitHub Actions: InspectionsLib/Gateway/Paypal.php[warning] 1-1: Silencing errors is strongly discouraged. Use proper error checking instead. Found: @Header( 'Content-Type: application/json; charset=' 🔇 Additional comments (6)
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).
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.
| $data['subtotal'] = $billing_amount - $tax_amount; | ||
| $data['subtotal'] = $subtotal; | ||
| $data['tax'] = $tax_amount; | ||
| $billing_amount = apply_filters( 'wpuf_payment_amount', $billing_amount, $post_id ); |
There was a problem hiding this comment.
- do not remove existing filter:
wpuf_payment_amount. - Tax is a pro feature. move all the tax related calculation to pro.
- send the tax separately to paypal. have a look at paypal doc
There was a problem hiding this comment.
done @sapayth vai, @Rubaiyat-E-Mohammad vai, please recheck this PR as we refactored the code.
sapayth
added
needs: author reply
Replaces direct tax calculation with filter-based approach for better compatibility with Pro's tax handling. Removes redundant tax rate retrieval and ensures tax breakdown is obtained via filters, aligning with Stripe's pattern and improving subtotal/tax breakdown accuracy for PayPal API.
arifulhoque7
added
needs: dev review
There was a problem hiding this comment.
Actionable comments posted: 1
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)
203-203: Address PHPCS violations flagged by the pipeline.The pipeline identified two code style issues:
- Line 203: Error suppression with
@operator is discouraged. Theheader()call should include proper error checking instead.- Multi-line function calls: Opening parentheses should be the last content on the line.
Please resolve these PHPCS violations to maintain code quality standards.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Lib/Gateway/Paypal.php
🧰 Additional context used
🪛 GitHub Actions: Inspections
Lib/Gateway/Paypal.php
[warning] 1-1: Silencing errors is strongly discouraged. Use proper error checking instead. Found: @Header( 'Content-Type: application/json; charset=' ...
[error] 1-1: Opening parenthesis of a multi-line function call must be the last content on the line. Command failed: vendor/bin/phpcs Lib/Gateway/Paypal.php includes/Ajax/Frontend_Form_Ajax.php -q --report=checkstyle | cs2pr --graceful-warnings
🔇 Additional comments (5)
Lib/Gateway/Paypal.php (5)
296-314: LGTM! Webhook validation properly rejects invalid data.The validation now explicitly checks that both
subtotalandtaxexist and are numeric before using them to compute the expected total. This addresses the previous concern about silently coercing missing or non-numeric values to 0. The error message includes both expected and received amounts for debugging.
335-337: LGTM! Correctly uses validated subtotal and tax from custom data.Using the validated
expected_subtotalandexpected_taxvalues fromcustom_dataensures the webhook processing uses the amounts that were calculated correctly at payment creation time.
1245-1251: LGTM! Using data-driven values for return URL is more reliable.Constructing the return URL with
$data['type']and$data['item_number']directly from the payment data array is cleaner and more reliable than deriving these from GET parameters.
1555-1557: LGTM! Improved error logging helps with debugging PayPal API issues.Adding detailed error information from PayPal's response to the exception message will make it much easier to diagnose payment failures.
1286-1314: Verify Bank.php compatibility or standardize tax handling across gateways.The refactored Paypal.php intentionally removes Pro's tax filter before applying
wpuf_payment_amount, expecting it to output subtotal-before-tax rather than total-with-tax. However, Bank.php (line 85) maintains the opposite expectation:$data['cost'] = apply_filters( 'wpuf_payment_amount', $data['price'], $post_id ); //price with tax from pro $data['tax'] = floatval( $data['cost'] ) - floatval( $data['price'] );Bank.php expects the filter to output total-with-tax, calculating tax as the difference. This same pattern is used throughout the codebase in Admin_Subscription.php, subscription.php, and Admin/Admin_Subscription.php.
This creates a breaking change: Third-party filters hooking into
wpuf_payment_amountwill malfunction when applied to the Bank gateway after being tested on Paypal, or vice versa. Either standardize tax handling across all gateways using the same approach, or update Bank.php and related files to align with Paypal.php's new pattern of delegating tax calculation to a separatewpuf_payment_tax_breakdownfilter.
arifulhoque7
added
needs: testing
and removed
QA Approved
Removes unused $tax_amount variable and updates tax checks and values to use $data['tax'] directly. Ensures correct tax handling in purchase unit breakdown.
sapayth
added
dev: on-progress
and removed
needs: dev review
|
Please provide zip for both the |
Rubaiyat-E-Mohammad
added
needs: dev review
3 participants
Fix: PayPal Double Taxation & Webhook Payment Recording Issues
Close issue
Related PRO PR
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.