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
Shipping Settings: Revert removal of default method names #41833
Conversation
Test Results SummaryCommit SHA: 2d8a192
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
Hi @chihsuan, @rodelgc, @moon0326, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
3 similar comments
Hi @chihsuan, @rodelgc, @moon0326, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Hi @chihsuan, @rodelgc, @moon0326, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Hi @chihsuan, @rodelgc, @moon0326, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
I wasn't able to test this because when I used WC 8.3.1, I saw the old shipping dialogs. If I use the live branch from the other PR, I'm also not allowed to leave the shipping method name empty |
Yes, sorry I wasn't clear on this @rjchow. You will create the shipping zone method using the old UI in WC 8.3. This will allow you to enter a Shipping Method using the default value for "Name" field. Then, update to this branch's WC and confirm no errors exist.
Indeed this is correct. The idea is to re-create a scenario where merchants would have created a method in the old dialog that has a default value, updated to the new, and seen errors due to missing "name" value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the updated test instructions it seems to work fine! Thanks
Manually apply changes from #41833 that were omitted
Submission Review Guidelines:
Changes proposed in this Pull Request:
In #40838 default names for WooCommerce Shipping Zone methods was removed in favour of displaying a placeholder. In the case below
e.g. Free shipping
. The removal of the default is causing issues with previously configured methods that relied on there being a default.It appears to have been a mistake to remove the default names of methods because systems interacting with shipping zone methods work under the assumption the default exists, and so this was a backwards incompatible change. This PR revert that changes and resets some unit and e2e tests back to their original state.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
/wp-admin/admin.php?page=wc-settings&tab=shipping
and see the "Shipping method(s)" column have names underneath.Changelog entry
Significance
Type
Message
Comment
Fixes a bug found in unreleased feature. No changelog needed.