Refactor a `cart/mergeCart`

[newsiberian]

Hello, I did some tests to this method and they show me that we can't rely on sessionId when merging carts.

Simple use case: You logged in on one device as normal user with items in cart [you have global sessionId=1 here]. Further you, or your son put some products in cart from other device and try to sign in into this account. On second device you, probably, will have sessionId=2, but server will search cart by sessionId=1. Carts, probably, will not be merged, because of that.

Second use case: I clean browser cache – sessionId changed, but server continue to search cart by sessionId=1.

SessionId is very unstable, and I think we should avoid of using it for such kind of tasks.

Currently I thinking about passing userId of a second device to this method by somehow and search second cart only by userId criteria.

Do you have any thoughts about this?

[aaronjudd]

sessionId should be unique for each new client (moz vs firefox, mobile vs desktop etc), but not unique in the same browser instance (tabs). It should also be unique on the server, as it's passed from the client.

See: https://github.com/reactioncommerce/reaction/issues/393

Let me describe the cart handling, as it was meant to work:

When an anonymous user visits, they get a sessionId, this a generated server side, and stored in the browser localStorage, so that combined with their anonymous user account identifies them within the tab/browser/window session. The presence of a unique sessionId tells us not to create another user account for an anonymous user. The same sessionId should never be used by the server for any two sessions, as it comes from the client.. it's only set on the server side in the initial subscription to Sessions.

If this same user visits on another device, browser or anonymous session they will get a new anonymous userId and sessionId.

When a visitor registers with a password or authentication service (FB,GH,etc), they are anonymous before they register. We then create a new user and account. If the user is authenticated, and has a sessionId that matches previous carts with the same sessionId, we then merge matching sessionId carts into the newly created cart. (and we should remove the existing anonymous cart, user, account).

We identify an authenticated user in Roles as a guest without the anonymous role.

When this authenticated user logs out of the site, the publication is updated and they no longer have permissions to view their authenticated cart. The publication sees this, and creates a new cart for them, with a new anonymous cartId, but still the same sessionId.

If the existing registered guest user adds items to an anonymous cart session, and then logs in, the anonymous cart will be merged to their existing authenticated cart with new items added, existing items with additional quantity will get incremented. This is true from any browser/session.

For reference, these are related open issues: https://github.com/reactioncommerce/reaction/issues/536 https://github.com/reactioncommerce/reaction/issues/398 https://github.com/reactioncommerce/reaction/issues/592

[newsiberian]

Hello, @aaronjudd, thanks for a great explanation. It was really helpful on a par with #393 to understand sessions. I see that we need to do few steps to solve this issue. I think we need to add a one, maybe more rules for logic that you described. There can be only one anonymous account per shop per session. Do you agree with this?

I thinking about connection of sessionId and user account. Currently they connected only through cart but not all anonymous accounts have cart, so, maybe we could add sessionId property to account object? It will be used only to control the population of anonymous accounts per session.

I tried to figure out all pros and cons of this approach. What do you think?

[newsiberian]

Hi, this:

I thinking about connection of sessionId and user account. Currently they connected only through cart but not all anonymous accounts have cart, so, maybe we could add sessionId property to account object? It will be used only to control the population of anonymous accounts per session.

was not the best idea :sweat_smile: There were small problems with cart publication and now it should be added to every anonymous user who enters the site. This step mostly passed.

Now there is a problem with this part. Tracker not rerun when I clean browser's cache and in fact there is no more sessionId in browser, but server side still remember previous sessionId and continue to use it.

I've just begun to look at this, but maybe you have some ideas where could be a problem here?

[aaronjudd]

There can be only one anonymous account per shop per session.

Yes, one anonymous user account per session (doesn't need to be shop specific).

Tracker not rerun when I clean browser's cache and in fact there is no more sessionId in browser,

I am not able to replicate this. Are you sure you are clearing localStorage?

Test with amplify.store("ReactionCore.session"). With a cleared cache it will be different every time. You will also see it creates a new user, account, cart.

[newsiberian]

here:

[aaronjudd]

ReactionCore.sessionId which is just server side, and is passed with the sessionId generated by the client. What you're seeing is not consistent with my tests using meteor shell. See: http://recordit.co/ggnI5XuK0K

[newsiberian]

We got a different results because of your "remove" button which, seems to me, did extra page reload, but default cleaning did not do that, as I can see...

[newsiberian]

this was fixed by #636. Could be closed, I suppose.