Tablerate Shipping regex not correct at RateQuery:134

delve-game-studios commented 5 years ago

Preconditions (*)

Magento version 2.3.2
UK zip codes are separated by space not a dash and the tablerate plugin doesn't recognize it.
File to be fixed: magento/module-offline-shipping/Model/ResourceModel/Carrier/Tablerate/RateQuery.php at line 134. It checks for prefixes from zip codes that are separated only by as dash but UK zip codes are separated by empty space.

Steps to reproduce (*)

Add records to the DB with some specific ZIP code prefixes from the UK
Try to input your address in the checkout page with a full ZIP code. be sure to input the correct format e.g: "AB31 2AR"

Expected result (*)

Show me the correct shipping method

Actual result (*)

There is no shipping method found.

Way of fixing (*)

Add the white space as an option in the regex.

"/^(.+)-(.+)$/" - Original Regex "/^(.+)(\s|-)(.+)$/" - Fixed Regex

This way it will get the correct prefix and will show the

m2-assistant[bot] commented 5 years ago

Hi @milan-vugrinchev. Thank you for your report. To help us process this issue please make sure that you provided the following information:

[ ] Summary of the issue
[ ] Information on your environment
[ ] Steps to reproduce
[ ] Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@milan-vugrinchev do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

[ ] yes
[ ] no

Join Magento Community Engineering Slack and ask your questions in #github channel.

m2-assistant[bot] commented 5 years ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
[ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Verify that the issue is reproducible on 2.2-develop branch.
Details
- Add the comment @magento give me 2.2-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x
[ ] 6. Add label Issue: Confirmed once verification is complete.
[ ] 7. Make sure that automatic system confirms that report has been added to the backlog.

heldchen commented 5 years ago

this should be a configurable option (with a sane default) IMO, as it makes too many assumptions that are valid for just one (US?) / a few countries currently.

engcom-Bravo commented 5 years ago

Hello @milan-vugrinchev, Thanks for applying. Would You please give us a reference to the official source on Table Rate shipping method in Magento where it is said that the rate of a particular record in the DB table will be applied to all zip-codes starting with a string given in dest_zip field of that record. Or anything else saying that dest_zip value is not necessarily supposed to be a valid zip-code in order for the according rate to be applied. Thank You.

delve-game-studios commented 5 years ago

Can’t really give anything of value just stating that this should be an issue because it’s not said that the TableRate shipping method for (for example US only method) that’s the idea behind my issue. UK zip codes start with String and are separated by space not a dash. I would only like this to be an official release instead of me touching vendor folder. That’s all.

Best Regards, Milan Vugrinchev

From: Alexander Potoky Sent: Wednesday, July 31, 2019 11:52 AM To: magento/magento2 Cc: Milan Vugrinchev; Mention Subject: Re: [magento/magento2] Tablerate Shipping regex not correct atRateQuery:134 (#23948)

Hello @milan-vugrinchev, Thanks for applying. Would You please give us a reference to the official source on Table Rate shipping method in Magento where it is said that the rate of a particular record in the DB table will be applied to all zip-codes starting with a string given in dest_zip field of that record. Or anything else saying that dest_zip value is not necessarily supposed to be a valid zip-code in order for the according rate to be applied. Thank You. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

engcom-Bravo commented 5 years ago

Thank You for replly @milan-vugrinchev. Seems there is some misunderstanding. If You store not the prefix but the valid UK zip-code in the DB (like AB31 2AR with a space in between, just as it should be) then everything will work properly. And as so then where the bug is?

engcom-Bravo commented 5 years ago

Or @milan-vugrinchev You mean there is no bug. You simply want Magento to be able to work with prefixes and not just with the full zip-codes?

delve-game-studios commented 5 years ago

Of course… otherwise we’ll have a 100GB table only for zip codes and for each zip code there’ll be at least 20 rows for price per package_value_with_discount … which makes this method inefficient. It has the functionality to check for prefixes but only one kind… not everywhere the zip code is separated from the prefix with a dash in some countries is a space.

Best Regards, Milan Vugrinchev

From: Alexander Potoky Sent: Wednesday, July 31, 2019 2:08 PM To: magento/magento2 Cc: Milan Vugrinchev; Mention Subject: Re: [magento/magento2] Tablerate Shipping regex not correct atRateQuery:134 (#23948)

Or @milan-vugrinchev You mean there is no bug. You simply want Magento to be able to work with prefixes and not just with the full zip-codes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

engcom-Bravo commented 5 years ago

Ok, @milan-vugrinchev, now everything has become clear. We are confirming Your issue. Thanks for reporting.

magento-engcom-team commented 5 years ago

:white_check_mark: Confirmed by @engcom-Bravo Thank you for verifying the issue. Based on the provided information internal tickets MC-18786 were created

Issue Available: @engcom-Bravo, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] commented 5 years ago

Hi @maheshWebkul721. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

[ ] 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 2. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 3. If the issue is not relevant or is not reproducible any more, feel free to close it.

Join Magento Community Engineering Slack and ask your questions in #github channel.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Thank you for your contributions.

magento-engcom-team commented 3 years ago

Unfortunately, we are archiving this ticket now as it did not get much attention from both Magento Community and Core developers for an extended period. This is done in an effort to create a quality, community-driven backlog which will allow us to allocate the required attention more easily.

Please feel free to comment or reopen according to the Issue reporting guidelines the ticket if you are still facing this issue on the latest 2.x-develop branch. Thank you for collaboration.

magento / magento2