Ticketpark / SaferpayJsonApi

A php library to use the Saferpay Json API
MIT License
32 stars 25 forks source link

Added serialized LanguageCode property with GET/SET methods #51

Closed mathmul closed 3 years ago

mathmul commented 3 years ago

Added as stated in the title. But it still doesn't really work. Let's debug?

mathmul commented 3 years ago

There are 11 errors given by PHPUnit. Let's focus on the first one.

1) Ticketpark\SaferpayJson\Tests\Request\PaymentPage\AssertRequestTest::testSuccessfulResponse
TypeError: Return value of Mock_Response_54e9c0c6::getBody() must be an instance of Psr\Http\Message\StreamInterface, string returned

/home/scrutinizer/build/lib/SaferpayJson/Request/Request.php:115
/home/scrutinizer/build/lib/SaferpayJson/Request/PaymentPage/AssertRequest.php:48
/home/scrutinizer/build/tests/SaferpayJson/Tests/Request/CommonRequestTest.php:60
/home/scrutinizer/build/tests/SaferpayJson/Tests/Request/CommonRequestTest.php:38
/home/scrutinizer/build/tests/SaferpayJson/Tests/Request/PaymentPage/AssertRequestTest.php:14

How does one even start fixing this?

I (obviously?) don't have access to Tests, so I can't check what exactly is being tested. All I know that there a getBody() method somewhere that returns a value of type string where it should be of type Psr\Http\Message\StreamInterface.

I checked Request.php and found $response->getBody() used twice, both times preceded with (string) which implicitly converts whatever the method return to a string type. But I don't suppose that's the problem.

Ctrl + click on the method itself leads us into GuzzleHttp dependency's trait MessageTrait in GuzzleHttp\Psr7 namespace where we can see the method. Skipping the conditional statement it returns $this->stream which is of type StreamInterface (or null), neither of which is a string so probably that's not the problem.

Let's check the conditional statement. It assigns Utils::streamFor('') to $this->stream. Checking out that method we see the first condition is true, but not the inner condition so it returns something of type Stream which implements StreamInterface so it's (and please hit me on the head now) the same enough ie. not a string.

Where did my inspection go wrong?

sprain commented 3 years ago

First of all, thanks for your pull request.

I now had the time to have a closer look at this. I noticed that languageCode actually already exists in the libary. It is part of the Payer container within the PaymentPage\InitializeRequest – as documented in the SaferpayJsonApi docs.

So the way to do it is like this:

$initializeRequest = new InitializeRequest(
    $requestConfig,
    $terminalId,
    $payment,
    $returnUrls
);

$payer = new Container\Payer();
$payer->setLanguageCode('fr');

$initializeRequest->setPayer($payer);

This means, the change in this pull request is not needed. I am sorry for not noticing earlier.

The failing tests were unrelated to these changes. I fixed them in #52.

mathmul commented 3 years ago

Aren't I a dummy?... Thank you very much! There will be another time for a first (accepted) PR :) Branch deleted.