Skip to content
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

Fix customer_details Backbone always setting Guest checkout True #2145

Closed
wants to merge 1 commit into from

Conversation

ericsaupe
Copy link
Contributor

When the page is rendered the value of #user_id is set and the Backbone
render was resetting that. This changes the render to fallback to that
value if the model doesn't have the user_id already present.

Steps to Recreate:
Create order in Admin
Get to address step
Use Customer Search to add a User to the order
Notice Guest Checkout is now No
Complete Address Step
Go back to Address Step
Guest Checkout is now Yes even though a user is attached to order

When the page is rendered the value of #user_id is set and the Backbone
render was resetting that. This changes the render to fallback to that
value if the model doesn't have the user_id already present.
@jhawthorn
Copy link
Contributor

Is it possible to get a regression spec for this?

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we fix the model instead? It should have an user_id if the order is not an guest order.

@ericsaupe
Copy link
Contributor Author

The model does have the user_id set. When going back that the Backbone JS it isn't initializing with that data.

@tvdeyen
Copy link
Member

tvdeyen commented Aug 21, 2017

Backbone JS isn't initializing with that data

Can we fix this then instead?

@ericsaupe
Copy link
Contributor Author

ericsaupe commented Aug 21, 2017

That's what this does. It either uses the data that has been set in Backbone through another method or gets the value from the rendered page while initializing. It could be done a different way. Probably in the initializer.

@cbrunsdon
Copy link
Contributor

We can make @swcraig write a regression feature spec for this.

@swcraig
Copy link
Contributor

swcraig commented Aug 21, 2017

I am working on the regression spec.

@cbrunsdon
Copy link
Contributor

Closing in favor of #2176 which includes the regression

@cbrunsdon cbrunsdon closed this Aug 22, 2017
@ericsaupe
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants