-
Notifications
You must be signed in to change notification settings - Fork 71
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 with tax and shipments #110
Comments
Does Sylius support multiple shipments? |
Hello Pierre and thank you for the issue. The problem is, plugin focuses on providing refund functionality for Sylius core. And indeed, Sylius allows multiple shipments per one order, but does not really support it - which means we usually assume there is one (or none) shipment on each order. In general, it does not provide any errors (order/checkout handling is quite straight-forward) but I can imagine it can be buggy in case of such a fragile feature as refunds. I leave it as a bug, it would be great if we can fix these issues without influencing current plugin logic too much. It will probably not be fixed by us in the nearest nearest future, but maybe during the stabilization phase after the release of Sylius 1.4 (hopefully 😄), we will be able to do something in this case. Of course, if you have some idea how to solve one or more of listed issues, feel welcomed to do that 🖖 |
Hello Mateusz, we have fixed them in our fork, but we have no time actually to PR them properly as long as we did some specific work in the fork (like fees on a refund). Feel free to pick them up here https://github.com/printoclock/RefundPlugin. About multi shipment, we dit not solve the problem completly as long as it will require some bigger modification to handle each shipment separately, but at least, it's no more wrong in calculation and taxes. |
@Zales0123, short list of what we do in our fork :
|
This PR was merged into the 1.0-dev branch. Discussion ---------- Partially fixes #110 Commits ------- 93e3fc5 [Behat] Add scenario for refunding shipment with a tax b71400e [Adjustment] Duplicate the code from Sylius for adding details to adjustment and making shipment adjustable b8c6045 Change order of migrations f63bd0f Add tax to refunding shipment 8d74d54 Change overriden entities from Sylius to be traits c29932d CS fixes 590f66a Minor refactor of foreach in OrderItemsTaxesApplicator 07782b1 Fixes in taxes applicators ce19c38 [Migrations] Use adding SQL instead of query execution
This issue is only partially fixed |
This PR was merged into the 1.0-dev branch. Discussion ---------- Fixes #252 and partially fixes (3rd point) #110 Commits ------- 4fa49c6 [Behat] Add scenario for refunding shipping cost with promotion applied b80aafa Refund shipping cost with promotion applied d5b8b63 [Behat] Add scenario for refunding shipping cost with 100% discount applied a874a70 Fixes according to static analysis
This PR was merged into the 1.0-dev branch. Discussion ---------- Fixes #110 Commits ------- 33e16be [Behat] Add scenarios for refunding multiple shipments a841673 [Taxation] Fix applying shipment taxes for multiple shipments 85b7959 Refunding multiple shipments 5092357 [Behat] Duplicate some steps from Sylius due to support for Sylius 1.8 a77277b [Taxation] Add spec for applying taxes when one of the multiple shipments has 0 tax amount
When calculating refunds on shipments, there are 3 bugs :
About which tax to apply.
For each unit you already rebuild tax from order item unit data (and not from variant/product/zone matching). That works but for shipments that's not working as long tax of each shipment is not stored per shipment adjustment but on a single order shipping adjustment, you should sum all shipment adjustement and calculate tax ratio with tax adjustment linked to the order to find which tax has been applied to a shipment.
(That will fail in case of multiple shipments with different tax on each shipments but I'm not sure it's really a use case in the context of a single order.)
The text was updated successfully, but these errors were encountered: