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

[Spree Upgrade] Provide anonymous user instead of nil #3247

Merged
merged 1 commit into from Jan 2, 2019

Conversation

Projects
None yet
3 participants
@sauloperez
Copy link
Contributor

sauloperez commented Dec 27, 2018

What? Why?

Closes #3078

As explained in https://github.com/openfoodfoundation/spree_auth_devise/blob/0181835fb6ac77a05191d26f6f32a0f4a548d851/app/models/spree/user.rb#L26-L32, all orders have a user so checking out as a guest is just using an automatically generated user by means of .anonymous!.

What should we test?

spec/features/consumer/shopping/orders_spec.rb:17 should be fixed.

Provide anonymous user instead of nil
As explained in
https://github.com/openfoodfoundation/spree_auth_devise/blob/0181835fb6ac77a05191d26f6f32a0f4a548d851/app/models/spree/user.rb#L26-L32,
all orders have a user so checking out as a guest is just using an
automatically generated user by means of `.anonymous!`.

@sauloperez sauloperez self-assigned this Dec 27, 2018

@sauloperez sauloperez force-pushed the coopdevs:fix-anonymous-checkout-spec branch from 375877e to 3e79365 Dec 27, 2018

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Dec 27, 2018

oh, you just force-pushed. is it ready for review? the first build looks good, the spec is fixed.

@sauloperez

This comment has been minimized.

Copy link
Contributor

sauloperez commented Dec 27, 2018

Yep, sorry I messed it with another branch. Now it's ready.

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Dec 27, 2018

rerunning build to see job 3 with 6 broken specs, not 8, should be random...

@mkllnk

mkllnk approved these changes Jan 2, 2019

Copy link
Member

mkllnk left a comment

Woohoo! Do you know what that means? We can solve the ugly problems around our Customer model! Once we have anonymous users for all guests, we can get rid of the email field. It opens up new solutions for inviting customers. 🎉

@mkllnk mkllnk merged commit 832dca9 into openfoodfoundation:2-0-stable Jan 2, 2019

1 check failed

semaphoreci The build failed on Semaphore.
Details
@sauloperez

This comment has been minimized.

Copy link
Contributor

sauloperez commented Jan 2, 2019

Gald to see you happy @mkllnk 😜. Sorry, what is it that we do with customer's email?

@mkllnk

This comment has been minimized.

Copy link
Member

mkllnk commented Jan 2, 2019

It's just that we have some data duplication around customer records in the database. Customers can be associated to an email (anonymous guest) and to a user id (registered user). Some of that is confusing and lead to bugs like #2841. Going with Spree's new way of having a user record for every guest could resolve some of these things.

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 3, 2019

@sauloperez sauloperez deleted the coopdevs:fix-anonymous-checkout-spec branch Jan 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment