contributte / gopay-inline

:credit_card: GoPay Inline Payment for Nette Framework
https://contributte.org/packages/contributte/gopay-inline.html
BSD 3-Clause "New" or "Revised" License
33 stars 32 forks source link

Missing getAccountStatement() #24

Closed haltuf closed 7 years ago

haltuf commented 7 years ago

In this version, I miss getAccountStatement() function to call this: https://doc.gopay.com/cs/#generov%C3%A1n%C3%AD-v%C3%BDpisu-obchodn%C3%ADho-%C3%BA%C4%8Dtu

f3l1x commented 7 years ago

It look's pretty simple to implement it, would you try it?

haltuf commented 7 years ago

Working on it.

haltuf commented 7 years ago

Well, not as easy as it looks on the first sight. API call /accounts/account-statement returns application/octet-stream When called, the Markette\GopayInline\Http\Response does not contain the content of the file, just headers. Any idea how to fix this and not break everything else?

f3l1x commented 7 years ago

Mmm. Can you check return header/content-type?

https://github.com/Markette/GopayInline/blob/master/src/Http/Curl.php#L34-L44

haltuf commented 7 years ago

object(Markette\GopayInline\Http\Response)#94 (4) { ["data":protected]=> NULL ["headers":protected]=> array(26) { ["url"]=> string(52) "https://gate.gopay.cz/api/accounts/account-statement" ["content_type"]=> string(24) "application/octet-stream" ["http_code"]=> int(200) ["header_size"]=> int(598) ["request_size"]=> int(379) ["filetime"]=> int(-1) ["ssl_verify_result"]=> int(0) ["redirect_count"]=> int(0) ["total_time"]=> float(0.232824) ["namelookup_time"]=> float(3.1E-5) ["connect_time"]=> float(0.008602) ["pretransfer_time"]=> float(0.039469) ["size_upload"]=> float(101) ["size_download"]=> float(60416) ["speed_download"]=> float(259492) ["speed_upload"]=> float(433) ["download_content_length"]=> float(-1) ["upload_content_length"]=> float(101) ["starttransfer_time"]=> float(0.224169) ["redirect_time"]=> float(0) ["redirect_url"]=> string(0) "" ["primary_ip"]=> string(13) "35.156.106.68" ["certinfo"]=> array(0) { } ["primary_port"]=> int(443) ["local_ip"]=> string(13) "88.86.109.181" ["local_port"]=> int(40482) } ["code":protected]=> int(200) ["error":protected]=> NULL }

The problem is in line https://github.com/Markette/GopayInline/blob/master/src/Http/Curl.php#L41

$result contains binary content of the file (xls/abo or csv). json_decode($result) then makes it NULL.

haltuf commented 7 years ago

My idea is to rewrite https://github.com/Markette/GopayInline/blob/master/src/Http/Curl.php#L40-44 to:

} else {

$info = curl_getinfo($ch);
$response->setCode(curl_getinfo($ch, CURLINFO_HTTP_CODE));
$response->setHeaders($info);

if($info['content_type'] == 'application/json') {
    $response->setData(json_decode($result));
} else {
    $response->setData($result);
}

}

What do you say?

f3l1x commented 7 years ago

Nice idea, that's what I was thinking about. Could we also check application/octet-stream content type? Just for sure.

haltuf commented 7 years ago

So like this?

if($info['content_type'] == 'application/json') {
    $response->setData(json_decode($result));
} elseif($info['content_type'] == 'application/octet-stream' {
    $response->setData($result);
} else {
    $response->setData($result);
}
f3l1x commented 7 years ago

At this moment we had just application/json. Your plan is to add a application/octet-stream. And I would like to throw an exception if neight of these conditions is mached. Do you agree or do you think it's pointless?

haltuf commented 7 years ago

I wouldn't throw an Exception as it poses a risk of breaking things, that currently work (we would have to check that all the current calls do return application/json correctly to be sure). Saying this, I would restate my suggestion to this:

if($info['content_type'] == 'application/octet-stream') {
    $response->setData($result);
} else {
    $response->setData(json_decode($result));
}

This way we make sure, that everything works exactly as now (and we don't break something by accident), with the exception of application/octet-stream that has newly added behaviour.

f3l1x commented 7 years ago

Ouk.

f3l1x commented 7 years ago

Resolved via #25