RobertSpread / spreadcart

simple plugin to display a cart for the spreadshop everywhere on your domain
1 stars 1 forks source link

Cart is not being updated after purchase #23

Open jtlapp opened 8 years ago

jtlapp commented 8 years ago

I made a purchase last night through the cart, and this morning I find that every item is still in the cart. On the other hand, the order is in "wait" status (I don't know why), so that might have something to do with it. In any case, we might want to revisit refreshing the cart after purchase.

Okay, I've just confirmed with a friend who made a successful order that the contents of her cart were not emptied after the order. That makes sense, because I don't see us doing that.

jtlapp commented 8 years ago

It's not clear to me how we tell that the cart has been emptied if the local storage isn't being updated. Maybe we always have to hit the API.

jtlapp commented 8 years ago

I'm porting the cart over to the API, so that the only thing it asks of local storage is the basket ID. I'm afraid of more surprises down the road, whether they are already latent or later a consequence of developer mods at Spreadshirt.

RobertSpread commented 8 years ago

I have time to do it tomorrow night. I can take this task over if you like

jtlapp commented 8 years ago

I'm almost done. It's a pretty major overhaul to do it in a way that isn't a hack. Thanks.

jtlapp commented 8 years ago

There appears to be a race condition. When an item is added to the cart and spreadCart is told it needs to get the new item from the API, the API cart has not yet finished updating for the new item by the time spreadCart is asking for it, so spreadCart just gets the old cart.

jtlapp commented 8 years ago

The solution that comes to mind is to respond to an add-to-cart click by setting a timer. At timeout, I read the cart via API. If the total quantity in the cart is not different at this time, set another timer and try again at that timeout. And have a maximum number of tries.

Interestingly, this solution can be generalized to remove more dependencies on the SpreadShop JS. It is possible to monitor for any click on a page that has SpreadShop. I could just set this sequence going for each click, with each subsequent click terminating any previous sequence and starting a series anew. This would also catch the user making adjustments via the SpreadShop-provided cart.

jtlapp commented 8 years ago

Alternatively, I could just update the cart every so many seconds and never monitor for clicks. I'd rather keep proxy traffic to a minimum, though.

jtlapp commented 8 years ago

There's also a hybrid solution (to the general problem of dependency on SpreadShop implementation). Every time the user clicks anything, I set a lengthy timeout (maybe 10 secs). Call that timer Z. While Z is active, I'll have another timer X periodically timeout (maybe every second). I'd refresh the cart via API on each X timeout. When Z times out, there are no more refreshes until the user clicks something again. This solution may allow the user to tolerate a longer interval between API requests.

jtlapp commented 8 years ago

I think I'll proceed with responding to a click by periodically reading the API until either I get a different item quantity or reach a maximum number of tries. I'll make it configurable whether it just does this for the SpreadShop add-to-cart button or for any click on a SpreadShop page. The latter configuration allows people to keep and use the SpreadShop cart if they desire (although not ideal). More importantly, it provides an out-of-the-box way to remove dependency on all SpreadShop implementation but the means for getting the basket ID, just in case something suddenly breaks on us later on.

jtlapp commented 8 years ago

Major problem. I have everything working beautifully, but because SpreadShop does not update for deleted items from the API, every time I add a new item, the entire cart returns to what it was prior to the deletion. SpreadShop is resubmitting its entire local cart, regardless of changes made via the API.

I'm not sure how to work around this. Everything else is working so well.

If I reload the page, SpreadShop reloads from the API and everything is right.

jtlapp commented 8 years ago

I need a way to force SpreadShop to reload the basket without requiring the page to reload.

jtlapp commented 8 years ago

I have a temporary solution. Closing the shopping cart reloads the page.

jtlapp commented 8 years ago

I made it much less onerous. Closing the shopping cart now only reloads the page if both SpreadShop is on the page and the user changed the something in the shopping cart.

jtlapp commented 8 years ago

I have something to check in, but you might want me checking this in on a branch other than master. It derives from your master branch, though. This new version now only reads the cart via API.

RobertSpread commented 8 years ago

Do you have the proxy.php also extended or "only" the node.js version? If I need to rework the php part I would prefer an individual branch so that we can merge both into the master.

jtlapp commented 8 years ago

I don't have PHP installed with any web servers right now, so yes, you'll need to create a new proxy action. Fortunately, it's as butt-simple as the delete action.

jtlapp commented 8 years ago

I need to fix one more thing before checking it in. After checking out with a basket, the basket ID becomes invalid. I need to properly handle the error that you get when you ask for the ID. I wonder how SpreadShop learns that a basket has been emptied and become invalid. If you return to the store from the checkout pages without checking out, the basket is still there. SpreadShop has to learn via API somehow. Relying on an error message on attempting to read the ID?

jtlapp commented 8 years ago

Okay Robert. I've tackled both the fragile dependency problem and the problem with spreadCart not learning that the shopping cart was emptied. It was roughly the same solution. This is largely a rewrite and won't merge well with anything but master. You'll want to create a new branch directly off of your master to accept my pull request, so you can revise the PHP proxy. Shouldn't require much. Here's my new branch:

https://github.com/jtlapp/spreadcart/tree/read-cart-from-api

You can see it in operation at http://Jopodia.com, which I have up and running again.

I look forward to seeing your work on coupons, but it'll probably take quite a bit of massaging to merge with this revision. Now I really have to go do other things for a while.

jtlapp commented 8 years ago

Ugh. It seems that my repository is still not recognizing your reverted code. It won't allow the merge without going through again to remove the newly added reverted code.

I don't understand this. When I compare my master to your master, it says there's nothing to do. Yet, I created my read-cart-from-api branch from my master, and it says there are conflicts with your master? Your master is supposed to be behind my branch. I don't get it.

jtlapp commented 8 years ago

Okay, I did the merge and pushed it as read-cart-from-api-2. It seems that all of my changes were conflicts. I'm not really sure how that keeps happening. Anyway, this branch will merge with your master (or rather, with whatever branch you create for it off of master):

https://github.com/jtlapp/spreadcart/tree/read-cart-from-api-2

Or you can just pull from my master branch. I was thinking that putting the merges in master would prevent this from happening again, but I see that's what I did last time.