Honor guest checkout config #36
Conversation
| return if spree_current_user or current_order.email | ||
| return if ( | ||
| spree_current_user || | ||
| (guest_authenticated? && Spree::Config[:allow_guest_checkout]) |
There was a problem hiding this comment.
Can this config check be folded into the body of guest_authenticated?, or is there a reason you kept them separate?
There was a problem hiding this comment.
My original thought was that guest_authenticated? shouldn't include the configuration check, however, it makes sense that guest_authenticated? should always return false when allow_guest_checkout is configured to false.
I went a bit further on the refactoring of this to make it more readable: 8bc711b
|
Hey Brian, Looking good! Just a couple small notes, but otherwise I think this is good to go. |
|
Don't know what's going on with CI here, tests pass locally. |
bbuchalter
force-pushed
the
honor-guest-checkout-config
branch
from
8bc711b to
0504447
Compare
The specs as written don't properly set the case for an authenticated guest, as an authenticated guest should have their email assigned to an order and therefore not need a redirect. If no email is assigned to the order, then a redirect is required.
Do not allow guest authenticated to proceed to checkout when guest checkout is not allowed.
The original implementation had two guard statements and was difficult to read because it lacked private methods which expressed intent. By consolidating the guard statements, the check_registration method is now reads clearly and the private methods which implement have a clear purpose.
|
We just switched this repo over to Travis CI; can you rebase to trigger a re-build, please? |
bbuchalter
force-pushed
the
honor-guest-checkout-config
branch
from
0504447 to
622fe09
Compare
|
Passed in Travis! |
|
👍 - thanks for fixing this! |
Do not allow guest authenticated to proceed to checkout when
guest checkout is not allowed.