Fix: load membership and customer with the correct helpers in PayPal Express IPN#1378
Open
vuckro wants to merge 3 commits into
Open
Fix: load membership and customer with the correct helpers in PayPal Express IPN#1378vuckro wants to merge 3 commits into
vuckro wants to merge 3 commits into
Conversation
… Express IPN In PayPal_Gateway::process_webhooks() the IPN `custom` field is "payment_id|membership_id|customer_id", but all three ids were loaded with wu_get_payment(), so $membership and $customer were either the wrong object type or false — leading to wrong-object data corruption (and a fatal when the later code calls Membership-only methods like is_active()/renew()). Also guard the index count so a malformed `custom` value doesn't raise undefined-index warnings. Use wu_get_membership() and wu_get_customer() for the respective ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PayPal gateway webhook processor now correctly parses the IPN custom field and retrieves the associated payment, membership, and customer entities using the appropriate lookup functions instead of incorrectly using the payment lookup for membership and customer indices. ChangesPayPal IPN Processing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
inc/gateways/class-paypal-gateway.php (2)
957-957: ⚡ Quick winCorrect the method reference in the comment.
The comment references
set_express_checkout(), but no such method exists in this file. Thecustomfield is actually built inprocess_checkout()at line 474.📝 Proposed fix
-// `custom` is built as "payment_id|membership_id|customer_id" (see set_express_checkout()). +// `custom` is built as "payment_id|membership_id|customer_id" (see process_checkout() line 474).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/gateways/class-paypal-gateway.php` at line 957, Update the inline comment that says the `custom` field is built by "set_express_checkout()" to correctly reference the actual builder function `process_checkout()` where `custom` is composed ("payment_id|membership_id|customer_id"); locate the comment near the `custom` usage and replace the incorrect method name with `process_checkout()` so the comment accurately points to the source of the `custom` value.
1700-1700: 💤 Low valueUnused variable: parsed
$customis never accessed.The
$customvariable is parsed from$body['PAYMENTREQUEST_0_CUSTOM']but is never used before the method returns$bodyat line 1702. This appears to be leftover or incomplete code.♻️ Suggested cleanup
Remove the unused line if it serves no purpose:
- $custom = explode('|', (string) $body['PAYMENTREQUEST_0_CUSTOM']); - return $body;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/gateways/class-paypal-gateway.php` at line 1700, Remove the unused parsing of PAYMENTREQUEST_0_CUSTOM in class-paypal-gateway.php: delete the line that assigns $custom = explode('|', (string) $body['PAYMENTREQUEST_0_CUSTOM']) in the method where $body is returned (ensure it is not used elsewhere in that method), or if the custom value was intended to be used, instead apply the parsed $custom to the appropriate $body field or downstream logic (locate the code in the class PayPal gateway method that currently returns $body and either remove the $custom assignment or integrate $custom into that method's returned data).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@inc/gateways/class-paypal-gateway.php`:
- Line 957: Update the inline comment that says the `custom` field is built by
"set_express_checkout()" to correctly reference the actual builder function
`process_checkout()` where `custom` is composed
("payment_id|membership_id|customer_id"); locate the comment near the `custom`
usage and replace the incorrect method name with `process_checkout()` so the
comment accurately points to the source of the `custom` value.
- Line 1700: Remove the unused parsing of PAYMENTREQUEST_0_CUSTOM in
class-paypal-gateway.php: delete the line that assigns $custom = explode('|',
(string) $body['PAYMENTREQUEST_0_CUSTOM']) in the method where $body is returned
(ensure it is not used elsewhere in that method), or if the custom value was
intended to be used, instead apply the parsed $custom to the appropriate $body
field or downstream logic (locate the code in the class PayPal gateway method
that currently returns $body and either remove the $custom assignment or
integrate $custom into that method's returned data).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6dea0d1-f7f3-4199-8b47-2a7ce67da456
📒 Files selected for processing (1)
inc/gateways/class-paypal-gateway.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In
PayPal_Gateway::process_webhooks()the IPNcustomfield ispayment_id|membership_id|customer_id, but all three ids were loaded withwu_get_payment(). So$membershipand$customerwere the wrong object type(or
false), leading to wrong-object data handling and a fatal when the latercode calls Membership-only methods (
is_active(),renew(), …).Changes
wu_get_membership()andwu_get_customer()for the respective ids.customvalue doesn't raiseundefined-index warnings.
Affects the legacy PayPal Express (NVP) gateway IPN path only.
Summary by CodeRabbit