Fixes two bugs in the Payping driver - #353
Open
alireza-hadizadeh wants to merge 2 commits into
Open
alireza-hadizadeh wants to merge 2 commits into
alireza-hadizadeh wants to merge 2 commits into
Conversation
mb_strtolower() on the raw JSON response lowercases keys before decoding, so $body only ever contains "paymentcode", never "paymentCode".
- Reads paymentRefId from the JSON `data` callback param (v3), not the old `refid` query param - Sends paymentCode and amount in the verify request, as required by v3 verification - Applies the same Toman conversion used in purchase() so the verify amount matches what was originally sent - Guards against a missing/malformed callback payload instead of fataling on a null property access
Member
|
please resolve the conflicts. sorry for that. we introduced a large upgrade to support php8.4+ and it caused many conflicts on your PR. |
Member
|
hey @alireza-hadizadeh , could you please resolve the conflicts and make sure tests pass ? otherwise I have to close the PR! |
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
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.
Description
Fixes two bugs in the Payping driver:
purchase()threwUndefined array key "paymentCode"on every successful transaction, because the response body's keys are lowercased before being read.verify()was still using the old v2 callback contract (readingrefidfrom the query string, and sending onlypaymentRefIdto the verification endpoint). Updated to match Payping's v3 API: readspaymentRefIdfrom thedatacallback payload, and sendspaymentCodeandamountalong withpaymentRefIdwhen verifying.Motivation and context
1.
purchase()— undefined array keyThe raw JSON response string is passed through
mb_strtolower()beforejson_decode():This lowercases the JSON keys along with the values, so a response like
{"paymentCode": "abc123"}becomes{"paymentcode": "abc123"}after decoding. The code then tries to access$body['paymentCode'](camelCase), which never exists in$body, causing:This happens on every successful purchase request (HTTP 200), since the error-handling branch is skipped and execution falls straight into the broken key access.
verify()already correctly used the lowercase$body['cardnumber'], which is consistent with this same behavior —purchase()was just missed.2.
verify()— outdated v2 callback contractPayping's v3 API changed how the payment reference is returned on callback and what the verify request body requires. The driver was still reading
refidfrom the query string and posting onlypaymentRefId. Per Payping's v3 documentation, the reference is now delivered as a JSON payload in thedataparam, and the verify request should also includepaymentCode(the transaction ID frompurchase()) andamount. The amount is converted to Toman using the same logic already used inpurchase(), so the value sent during verification matches what was originally sent during purchase. A null-check was also added around the incoming callback data, since a missing/malformed payload previously caused an unhandled fatal error instead of a clean exception.Note for reviewers: the
verify()change alters the callback contract (source ofrefIdmoves from query string to thedatapayload). If any existing integrations relied on the oldrefidquery param, this could be a breaking change for them depending on how Payping's callback behaves for accounts still effectively on the old flow — flagging this explicitly so maintainers can weigh in on whether it needs a major version bump or should support both formats during a transition period.How has this been tested?
Manually tested against the live Payping purchase and verification endpoints in a Laravel app using this package.
paymentCode(camelCase) before lowercasing, and that using$body['paymentcode'](lowercase) resolves the transaction ID correctly.dataJSON payload per Payping's v3 docs, and that sendingpaymentRefId,paymentCode, andamounttogether completes verification successfully.Screenshots (if appropriate)
N/A
Types of changes
Checklist: