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

Update Deserializes.php Timezone problems #746

Merged
merged 5 commits into from
Jul 9, 2024

Conversation

simonsolutions
Copy link
Contributor

Fix assuming the wrong timezone when converting DateTime values ending with "Z".

Fix assuming the wrong timezone when converting DateTime values ending with "Z"
Copy link
Owner

@jlevers jlevers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test would be great :) preferably on the endpoint where you found the error!

src/Traits/Deserializes.php Outdated Show resolved Hide resolved
@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

@simonsolutions happy to merge this as soon as you push a test. A good test for this would check if a datetime string with a timezone specifier (like +02:00) is deserialized into a DateTime object with that same timezone, and check if a UTC datetime string is deserialized into a DateTime object set to UTC. Thanks :)

Add deserialize test with Timezone
@simonsolutions
Copy link
Contributor Author

Now added a simple deserialize sample with timezone.

tests/SerializationTest.php Show resolved Hide resolved
Add additionally checking the right timezone
tests/SerializationTest.php Outdated Show resolved Hide resolved
Sorry missed a bracket
Copy link
Contributor Author

@simonsolutions simonsolutions left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added missing bracket

tests/SerializationTest.php Show resolved Hide resolved
@jlevers jlevers merged commit 286be85 into jlevers:main Jul 9, 2024
2 checks passed
@simonsolutions simonsolutions deleted the patch-2 branch July 9, 2024 18:58
@zaineco
Copy link

zaineco commented Aug 5, 2024

API for deliveryWindowOptions returning date as 2024-08-12T00:00Z which causes the unknown DateTime format exception.
Please also add 'Y-m-d\TH:i\Z' as supported format in src/Traits/Deserializes.php

@jlevers
Copy link
Owner

jlevers commented Aug 5, 2024

@zaineco could you share a debug log from calling the endpoint that's giving you the exception? Just call ->debug() on the connector instance you're using to get the debug output.

@zaineco
Copy link

zaineco commented Aug 6, 2024

@jlevers, the following is response log for ListDeliveryWindowOptions endpoint

Saloon Response(ListDeliveryWindowOptions) -> array:3 [
  "status" => 200
  "headers" => array:12 [
    "Server" => "Server"
    "Date" => "Tue, 06 Aug 2024 07:50:50 GMT"
    "Content-Type" => "application/json"
    "Content-Length" => "3973"
    "Connection" => "keep-alive"
    "Vary" => "accept-encoding,Content-Type,Accept-Encoding,User-Agent"
    "Strict-Transport-Security" => "max-age=47474747; includeSubDomains; preload"
  ]
  "body" => "{
                "pagination":{
                   "nextToken":"aafd8957-77f9-434c-b432-3277b795156d"
                },
                "deliveryWindowOptions":[
                   {
                      "availabilityType":"AVAILABLE",
                      "endDate":"2024-08-12T00:00Z",
                      "validUntil":"2024-09-24T00:00Z",
                      "deliveryWindowOptionId":"dwf5ab76c1-ed8a-4885-beb6-3a9992b6954e",
                      "startDate":"2024-08-05T00:00Z"
                   },
                   {
                      "availabilityType":"AVAILABLE",
                      "endDate":"2024-08-13T00:00Z",
                      "validUntil":"2024-09-24T00:00Z",
                      "deliveryWindowOptionId":"dwc94fe632-512d-40a3-831e-b171289ce238",
                      "startDate":"2024-08-06T00:00Z"
                   },
                   {
                      "availabilityType":"AVAILABLE",
                      "endDate":"2024-08-14T00:00Z",
                      "validUntil":"2024-09-24T00:00Z",
                      "deliveryWindowOptionId":"dwa7a8bcea-915b-4f89-aefc-58b8a7ad367f",
                      "startDate":"2024-08-07T00:00Z"
                   }
                ]
             }"
]

@jlevers
Copy link
Owner

jlevers commented Aug 6, 2024

Thanks – that should be fixed in v7.1.1.

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