-
Notifications
You must be signed in to change notification settings - Fork 207
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
packages(package) doesn't work as intended when item weights are zero #187
Comments
I agree that packages logic is messed up, if you do a pr just make sure to take into consideration the weight conditions on services, some depend on nil or 0 values, nil means it shouldn't pass and 0 means there is no restriction. I cleaned this logic a bit and removed what we felt was not doing much for us here feel free to go through it and comment where needed, we use this in production and are getting good results, |
As I'm looking into a PR, I'm realizing that this is even thornier than I thought originally. For instance, if I'll leave the above as an issue, and leave the PR to folks who know the code and its intention better than I do. |
Agreed, I think the package weight restrictions should be enforced by splitters when creating shipments, and not this late when you already have the shipments and just need their rates. |
I've been looking closely at the logic of the
packages(package)
method, and I'm seeing some weird behavior when product weights are zero.If a product weight is zero and
max_weight
is zero, then it will go into a package.If a product weight is zero and there are other products that are weight > 0, then it will get lumped in with them into a package.
However, if there are only zero-weight products, then they will not get into a package due to the conditional in https://github.com/spree/spree_active_shipping/blob/master/app/models/spree/calculator/shipping/active_shipping/base.rb#L236
However, fixing this problem isn't as simple as removing that conditional, because we might end up with nil packages getting tacked on after the
weights.each
block.I'll prepare a PR with my proposed solution, which is to set package_weight to nil before the block and use a conditional on
weight != nil
rather than onweight > 0
.The text was updated successfully, but these errors were encountered: