search

sveawebpay / php-integration

SDK for Sveas payment methods (standalone and Svea Checkout)
Other
15 stars 19 forks source link

doRequest() refactor in 3.6.2 breaks GetAddresses #83

Closed fredrikwillers closed 6 years ago

fredrikwillers commented 6 years ago

doRequest refactor causes a 500 error for us when using GetAddresses

https://github.com/sveawebpay/php-integration/commit/1b426ee2c84ea4ab6f35b60d4fe9c59d542313a9#diff-692c52686473b2267534fd949720a64dR168

We now rolled back to previous versions but please have a look or let us know if we need to reconfigure anything?

fre-sund commented 6 years ago

That's odd. I just ran the example with the latest version using GetAddresses and I got no error. The test hasn't been changed since about 2 years.

Can you send me an example of the code you're running and which version you previously used?

Also all the unit & integration tests passed, and they haven't been changed for quite a while either.

fredrikwillers commented 6 years ago

@fre-sund thanks for a quick reply!

All the tests are explicitly setting the order type, we are however letting the checkAndSetConfiguredPaymentMethod implicitly set the order type based on the credentials.

fre-sund commented 6 years ago

Ok, I see the problem. We need to make some new tests regarding that case and update the library. I will try to reproduce the issue tomorrow. Have nice day!

fre-sund commented 6 years ago

Fixed in 3.6.3

Sorry for any problems that this caused you, I've added an integration test for when the order type isn't set so that we don't accidently break this again.

Have a nice weekend!

lswinblad commented 6 years ago

@fre-sund https://github.com/sveawebpay/php-integration/commit/d75d69cf98899e2f7cf57ca5207dc6863b6d0d0d#diff-78be8d144aa3290500a27414790ea02dR161 this change looks a bit weird?

fre-sund commented 6 years ago

@Farskies can you elaborate? If prepareRequest is called before doRequest the order type will be set and won't cause an exception.

fre-sund commented 6 years ago

Nvm just saw it.

fre-sund commented 6 years ago

@Farskies It's fixed now, apparently no errors occur when calling "$this->" on a separate line in that specific case. All tests did pass but I removed it since it looks odd and it basically does nothing.