search

postnl / postnl-magento2

This is the official Magento 2 extension for the logistics company PostNL. Add shipping options and parcelshops to your checkout. Create labels with track and trace functionality from the backend.
58 stars 61 forks source link

[BUG] Incorrect table rate due to incorrect sorting #293

Closed mlaurense closed 2 years ago

mlaurense commented 2 years ago

When filtering the table rates, the matrixrateCollection is sorted using only the subtotal (descending). But this breaks the correct rate calculation when using weight. Given a cart of ~ 35kg of weight, after filtering the rates in vendor/tig/postnl-magento2/Service/Carrier/Price/Matrixrate.php, these two rates are remaining:

Schermafbeelding 2022-03-08 om 10 42 14

This will incorrectly return a shipping of € 5, while it should return € 10.

Possibly related to https://github.com/tig-nl/postnl-magento2/issues/224 as well

To Reproduce Steps to reproduce the behavior:

  1. Create two rates using the example in the screenshot
  2. Add enough items to the cart so the weight of the second rate should be used
  3. See it return the lower of the two rates

Expected result Shipping should be € 10

Actual result Shipping is € 5

Workaround Modify https://github.com/tig-nl/postnl-magento2/blob/master/Service/Carrier/Price/Matrixrate.php#L97 to also sort by weight: $matrixrateCollection = $this->matrixrateCollection->addOrder('subtotal', 'DESC')->addOrder('weight', 'DESC');

After filtering, the rates are returned as follows:

Schermafbeelding 2022-03-08 om 11 18 19

Please complete the following information

tig-jeffreybranderhorst commented 2 years ago

Hi @mlaurense,

Thank you for submitting this issue with this much information. We are going to test it, I do have some questions. You are using the matrix rates not the table right, am I correct? And would it be possible to share your matrix rate, so we are testing with the same rates?

Have a great day. Jeffrey

mlaurense commented 2 years ago

Hi @tig-jeffreybranderhorst,

Yes, it is matrix rates (my bad). I attached the used rates:

matrixrate-test.csv

tig-jeffreybranderhorst commented 2 years ago

Hi @mlaurense,

Thank you for the matrix rates we are going to test this!

Have a great day, Jeffrey

tig-jeffreybranderhorst commented 2 years ago

Hi @mlaurense,

We have tested your matrix, and we can reproduce the last price is not reached because the 50 and above comes first, and then he will stop there instead of going to the 75. We will test your PR thank you for submitting this!

Have a great day, Jeffrey

tig-vincentthart commented 2 years ago

Hi @mlaurense ,

We have just released v1.12.2 and included a fix for this. Thank you for the detailed description.

Have a nice day! Vincent