Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response requestDate wrong #70

Closed
cottton opened this issue Aug 31, 2019 · 4 comments
Closed

Response requestDate wrong #70

cottton opened this issue Aug 31, 2019 · 4 comments

Comments

@cottton
Copy link
Contributor

cottton commented Aug 31, 2019

Soap response:

'requestDate' => '2019-08-31+02:00',

means you got Y-m-dP

P
Difference to Greenwich time (GMT) with colon between hours and minutes (added in PHP 5.1.3)
Example: +02:00

see https://www.php.net/manual/de/function.date.php

You are currenty using Y-m-d\+H:i which set it to 2am always :)

So in example:
response:
'requestDate' => '2019-08-31+02:00',

current code:
date_create_from_format('Y-m-d\+H:i', $response->requestDate)
returns:

DateTime::__set_state(array(
    'date' => '2019-08-31 02:00:00.000000',
    'timezone_type' => 3,
    'timezone' => 'UTC',
))

should use:
date_create_from_format('Y-m-dP', $response->requestDate)
BUT
this would automatically add current time due to the api does not provide time.

So a solution would be (since we get no time) to set the time to zero.

$datetime = date_create_from_format('Y-m-dP', $response->requestDate);
$datetime->setTime(0, 0, 0, 0);

DateTime::__set_state(array(
    'date' => '2019-08-31 00:00:00.000000',
    'timezone_type' => 1,
    'timezone' => '+02:00',
))

PR: #69

EDIT: the warning in te main readme should be removed.
WARNING: VIES service returns invalid time, use your own time setting!

PR: #71

EDIT: i just saw you have the format already implemented but not used anywhere:
\DragonBe\Vies\CheckVatResponse::VIES_DATETIME_FORMAT
public const VIES_DATETIME_FORMAT = 'Y-m-dP';

gonna change that ...

@DragonBe
Copy link
Owner

Yes, the time issue has been a pain since the beginning, but I never got around to fix it. Thank you for tackling it.

Would you also run the unit tests as well and see how this change impacts current logic?

@cottton
Copy link
Contributor Author

cottton commented Aug 31, 2019

PR: #72 (remove old)
PR: #73 (add new way to set date)

@DragonBe
Copy link
Owner

Thank you for the PR.

I’m not at home at the moment so I can’t merge it in just yet. I will have time for it tonight.

@DragonBe
Copy link
Owner

DragonBe commented Sep 1, 2019

Yesterday I was not able to have a look at it, today I did but ran into some issues regarding our tests. I've fixed the issues and got everything merged in.

Again, thank you for your contribution 👍

@DragonBe DragonBe closed this as completed Sep 1, 2019
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

No branches or pull requests

2 participants