Skip to content

Conversation

@parzerphilipp
Copy link

make $params protected to add the possibility to override the PaymentRequest class.
for example: if you want to add a parameter to the URL (e.g. USERPARAM1 for recurring payments), currently this cannot be done

make params protected
@newPOPE
Copy link
Owner

newPOPE commented May 15, 2023

What about method addParam or setParam for this? Instead of open visibility.

@parzerphilipp
Copy link
Author

would be a fine alternative solution

@newPOPE
Copy link
Owner

newPOPE commented May 15, 2023

Cool, will be merged after update.

make params private again
add setParam function
@newPOPE newPOPE merged commit b6816cd into newPOPE:master May 15, 2023
@parzerphilipp parzerphilipp deleted the patch-1 branch May 16, 2023 05:07
@romanklabal
Copy link

I am afraid it won't be as simple as that. If you for example do $request->setParam('LANG', 'cs');, then the LANG param will be included in the array returned by the getParams() method and the request will fail with PRCODE=31, SRCODE=0, because the LANG param is not supposed to be included in the digest.

@newPOPE
Copy link
Owner

newPOPE commented May 17, 2023

@romanklabal so solution can be, prepare params for digest alone, via some filter out access?

@romanklabal
Copy link

@newPOPE looking at the documentation (page 8), the LANG param seems to be the only one that should not be present in the digest. So maybe all you need to do is discard the LANG param when making the digest and only use it when building the URL. One of the forks already does it: https://github.com/l4ngur/gp-webpay-php-sdk/blob/master/src/AdamStipak/Webpay/PaymentRequest.php#L76

@newPOPE
Copy link
Owner

newPOPE commented May 17, 2023

@romanklabal what about this 0289bcb ?

@romanklabal
Copy link

I like it!

@parzerphilipp
Copy link
Author

we got one more problem
if I use the setParam function the params in the createPaymentRequestUrl() function have a different order than requested
for example USERPARAM1 is required before PAYMETHOD, but is added to the params via the setParam method and therefore after PAYMETHOD in the constructor
btw. it already fails if you add the MD param via constructor, because its added after the PAYMETHOD

so in my view, we have three possibilities:

  1. add a sorting function with all param names
  2. add all possible params in the constructor, in the correct order
  3. make params protected, so that the constructor can be overwritten if additional params are required

@newPOPE
Copy link
Owner

newPOPE commented May 19, 2023

@siwa-pparzer hi, can you code a test case with problematic scenario. I can help you with implementation afterwards.
I just maintain this lib now and don't using it in any project so it is a little bit tricky understand what it needs to be implemented. Test case will help.

@parzerphilipp
Copy link
Author

parzerphilipp commented May 21, 2023

$request = new PaymentRequest(time(), 100, PaymentRequest::EUR, 0, 'https://www.test.com', 1, null, null, PaymentRequest::PAYMENT_CARD);
$request->setParam('USERPARAM1', 'R');
$url = $this->api->createPaymentRequestUrl($request);

As you can see, the PAYMETHOD (PaymentRequest::PAYMENT_CARD) is set via constructor.
Then, I add the param USERPARAM1 (R) to the initial request.
But in the documentation (https://www.gpwebpay.cz/downloads/GP_webpay_HTTP_EN.pdf page 6-7) it states, that USERPARAM1 has to be declared before PAYMETHOD.
The creation of the url works, but when I call the url, it says, that the digest is not correct.
When I add the following static statement into the constructor before settings PAYMETHOD, it works:

$this->params['USERPARAM1'] = 'R';

@parzerphilipp
Copy link
Author

could be something like this:
#16

@parzerphilipp
Copy link
Author

did you check this already?

@newPOPE
Copy link
Owner

newPOPE commented Dec 7, 2023

Ou sorry, missed a notification before.

So, I read comments before and I'm correct understand the problem solution (sound fine for me is a 1. add a sorting function with all param names) can you add it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants