Assign default user addresses in checkout controller #1967
Merged
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
It fixes solidusio#1811: when cart -> address state change happens and user is not yet logged in, the `assign_default_addresses!` callback will not associate any address to the order regardless if the user will immediately later perform a login and had some address associated. By calling `assign_default_addresses!` method in the `before_address` checkout controller's callback we'll be sure we always set addresses correctly before entering the address step.
kennyadsl
force-pushed
the
assign-addresses-before-address-step
branch
from
337af88 to
f13d067
Compare
kennyadsl
changed the title
|
@jhawthorn @mamhoff this is not going to change any value on a GET request. It just sets the address without saving. |
mamhoff
approved these changes
Jun 14, 2017
|
Thanks @kennyadsl, I bumped into this problem just the other day. Can confirm this resolves the issue 😀 |
jhawthorn
approved these changes
Jun 14, 2017
|
That's great! Thanks so much! It's those small usability things, which are most annoying. Thanks again! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
It fixes #1811:
When cart -> address state change happens and user is not yet logged in, the
assign_default_addresses!callback will not associate any address to the order regardless if the user will immediately later perform a login and had some address associated.By calling
assign_default_addresses!method in thebefore_addresscheckout controller's callback we'll be sure we always set addresses correctly before entering the address step.The only cons I see is that we call this method much often (every time we hit the checkout address page) but it should be safe since, as @aaronrussell pointed out, it does nothing when the order has no user or when the order already has addresses associated.
I'm not sure if we want to remove the state machine callback now and move this functionality at a controller level, but we'll probably want to add
assign_default_addresses!to backend and api controllers as well.This PR also adds checkout feature specs that allows to reproduce this issue.