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

Bug Fix for GiftCard.ExpiresOn #533

Merged
merged 3 commits into from
Jul 23, 2020

Conversation

ChaoticIke
Copy link
Contributor

@ChaoticIke ChaoticIke commented Jul 2, 2020

When creating updating a gift card with an expiry date, the value is ignored because the date format needs to be 'yyyy-MM-dd'

The Date Format Converter takes a format parameter since it comes from one of my own projects. Let me know if you want to change it.

@ChaoticIke
Copy link
Contributor Author

image
I had some trouble getting to run the tests using the Environment Variables. I just hard coded my Shopify credentials for the tests.

It seems that the Create API doesn't accept an ExpiresOn, so I have changed the test to update instead.

I also needed to change the search test since searching by code isn't supported in this version of the API

@nozzlegear
Copy link
Owner

Sorry, I thought I had merged this and published it but it seems I did not!

@nozzlegear nozzlegear merged commit fb7761a into nozzlegear:master Jul 23, 2020
@nozzlegear
Copy link
Owner

Alright, published in 5.4.0 on Nuget. Thank you for the pull request!

@ChaoticIke ChaoticIke deleted the GiftCardExpiry branch July 23, 2020 09:19
@clement911
Copy link
Collaborator

@ChaoticIke Shouldn't the ExpiresOn be a DateTime instead of a DateTimeOffset then? Since it doesn't have any time information...

@ChaoticIke
Copy link
Contributor Author

@clement911 I thought about that as well. I wasn't sure if you would want to introduce a breaking change to fix a serialisation issue, so I left the data types as they were.

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