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

Fix for shipping calculators to return integers #2463

Merged
merged 1 commit into from
Feb 14, 2015

Conversation

Dukecz
Copy link
Contributor

@Dukecz Dukecz commented Feb 13, 2015

Fix for shipping calculators to return integers because of error thrown during checkout caused by #2427

@pjedrzejewski
Copy link
Member

Amount should be integer already, that's strange.

@Dukecz
Copy link
Contributor Author

Dukecz commented Feb 13, 2015

Its string. I have just updated to from 0.12 to 0.13 and run into that problem. Other shipping calculators has int casting already.

pjedrzejewski pushed a commit that referenced this pull request Feb 14, 2015
Fix for shipping calculators to return integers
@pjedrzejewski pjedrzejewski merged commit d672cf0 into Sylius:master Feb 14, 2015
@pjedrzejewski
Copy link
Member

Thanks @Dukecz!

@michalmarcinkowski
Copy link
Contributor

@Dukecz can you describe the issue? All scenarios passed, so we definitely should also cover the one you encountered to prevent future bugs.

@Dukecz
Copy link
Contributor Author

Dukecz commented Feb 16, 2015

@michalmarcinkowski Updated to sylius 0.13 and made testing order selecting flat rate shipping method, that was created before update.
After that the error has been thrown because price returned by shipping was "0" (string) instead of 0 (int).

@michalmarcinkowski
Copy link
Contributor

Ok, I understand. You had strings saved in the database, what is wrong and will not happen from v0.13, but after your fix we will be backward compatible. @Dukecz thanks for making the point!

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.

None yet

3 participants