This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Checkout Block: Prevent changes in the selected shipping method when new rates are added or removed #10457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
tarunvijwani
added
type: bug
tarunvijwani
changed the title
woocommercebot
requested review from
a team and
opr
and removed request for
a team
opr
suggested changes
Aug 11, 2023
There was a problem hiding this comment.
Hi, I tested and ran into an issue, not sure if this is intentional behaviour but maybe look into it?
- Enable Multiple Packages for WooCommerce and Go to WooCommerce > Settings > Multiple Packages, set the "group by" option to "Product (individual)"
- Have shipping methods set up for a specific location
- Do not have shipping methods set up for "other" locations.
- Add 2 items to the cart and go to Checkout.
- Enter an address where no shipping options exist.
- Select Local Pickup.
- Reload the page, notice local pickup is still selected
- With Local Pickup still selected, change address to one covered by a shipping zone.
- Notice Local Pickup is still selected
- Reload the page, notice Local Pickup is no longer selected
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
github-actions
bot
removed
the
status: stale
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
|
We've done more research and discovered that WooCommerce has a default behaviour that resets shipping methods every time an address change occurs. However, we have overridden this behaviour using a filter. I'll update the unit tests for the PR shortly and get it ready for review as soon as I can. Internal discussion: p1693292016236569-slack-C8X6Q7XQU |
github-actions
bot
removed
the
status: stale
ProblemIf local pickup is enabled but there are no available shipping methods for a specific address, we have noticed that the shipping method will change automatically to the default option that is available first once the shipping address is modified. This issue occurs because WooCommerce Core resets shipping methods whenever the address changes. Solution 1When the address is changed during checkout, it calls the "cart/update-customer" endpoint. Within this endpoint, the boolean value "prefersCollection" is passed as "true" if the customer has selected the "local pickup" option. This value then overrides the selected shipping rate with the available shipping rates. You can find the relevant code for this process here: 4660933 However, this process works only if the shopper remains on the same page. If the shopper navigates away from the checkout page and returns, the first available shipping method will be selected instead of the one previously chosen. This issue occurs because we only modify the selected shipping rate after the address changes. The value in "wc()->session->get( 'chosen_shipping_methods', array() );" remains the same. This causes the "prefersCollection" value to be false when the page loads for the first time or gets refreshed. Solution 2There is a way to override the behavior of Woo Core that resets the chosen shipping method. This is done by implementing a "woocommerce_shipping_chosen_method" filter. However, using this filter caused some of the existing unit test cases for store API to fail. Additionally, I have observed that the filter does not always work when the page is refreshed. Solution 3Along with Solution 1, manually update the 'chosen_shipping_methods' session data when the shipping address changes. @opr @mikejolley I followed Solution 3 in this PR and manually override the session data. However, it's a hacky workaround, and there seems to be no other solution. I have invested significant time into this PR, so I'm marking this PR as ready for review. However, I am also interested in exploring alternative solutions if available. I'm up for the pairing sessions. Let me know. |
|
Based on the testing steps I am confused as to why so much code, particularly in store API, is needed to solve this. The simplest case to test this is:
In trunk this reverts selection from local pickup to shipping. But this can be manipulated via 1 filter on core to determine the "default" selected rate. Adding this to ShippingController in blocks: add_filter(
'woocommerce_shipping_chosen_method',
function ( $default, $package_rates, $chosen_method ) {
// If we had a chosen method, and it still exists, return it. This prevents the selection changing if new
// rates are added or removed.
if ( $chosen_method && isset( $package_rates[ $chosen_method ] ) ) {
return $chosen_method;
}
// Otherwise pass through new default.
return $default;
},
20,
3
);@tarunvijwani did you try this approach? |
tarunvijwani
force-pushed
the
fix/9790-shipping-method-switch
branch
from
d9c2ca7
to
ecf2d33
Compare
tarunvijwani
changed the title
|
@mikejolley I tried this approach and shared the Slack earlier: p1694001430672649/1693292016.236569-slack-C8X6Q7XQU However, it seems that I wasn't using it correctly, so it was causing some tests to fail. Thank you so much for sharing the correct approach. It works great! 🎉 Updated the PR ✅ |
opr
approved these changes
Sep 18, 2023
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
|
@opr Thank for reviewing the PR.
I forgot to untick the box when I changed the code and description of the PR. I don't think we will need unit test cases for the current changes in the PR because we are not altering any store API anymore. |
Happy to see this merged. For long-lived branches like this one let's refrain from force pushing. There is a lot of context that has been lost. |
|
@ralucaStan Sure, I will avoid force pushing the code. 👍 |
5 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9790
If local pickup is enabled and there are no available shipping methods for a particular address, we have observed that the shipping method will automatically switch to the default option once the shipping address is modified and the shipping method is available for the new address.
With this PR, we're preventing the switch to the default shipping method address from being changed.
Changes in the PR
woocommerce_shipping_chosen_methodfilter to prevent changes in the selected shipping method.Testing
Automated Tests
User Facing Testing
WooCommerce Visibility
Changelog