search

vuestorefront / vue-storefront-1

The open-source frontend for any eCommerce. Built with a PWA and headless approach, using a modern JS stack. We have custom integrations with Magento, commercetools, Shopware and Shopify and total coverage is just a matter of time. The API approach also allows you to merge VSF with any third-party tool like CMS, payment gateways or analytics. Newest updates: https://blog.vuestorefront.io. Always Open Source, MIT license.
https://www.vuestorefront.io
MIT License
19 stars 13 forks source link

Add to cart broken for new users with server sync on #359

Open AgarvaiN opened 4 years ago

AgarvaiN commented 4 years ago

Current behavior

When a user tries to add a product to the cart from a new browser (or one with data cleared), where he doesn't already have a cart. Product gets added for a second, then it gets instantly removed. After the button is clicked the second time, it adds the product fine.

Expected behavior

User should be able to add a product to the cart with a single click.

Steps to reproduce the issue

Clone VSF from Master Copy default.json to local.json In local.json just update the cart section to look like this

"cart": { "thumbnails": { "width": 150, "height": 150 }, "bypassCartLoaderForAuthorizedUsers": false, "serverMergeByDefault": true, "serverSyncCanRemoveLocalItems": true, "serverSyncCanModifyLocalItems": true, "synchronize": true, "synchronize_totals": true, "setCustomProductOptions": true, "setConfigurableProductOptions": true, "askBeforeRemoveProduct": true, "displayItemDiscounts": true, "productsAreReconfigurable": true,

Tried with default theme, and demo.vuestorefront.io as API

Run the app Open the browser in Incognito or Clear Application Data and reload the page Try to add product to cart

Can you handle fixing this bug by yourself?

Which Release Cycle state this refers to? Info for developer.

Pick one option.

Environment details

Additional information

Important setting "serverSyncCanRemoveLocalItems": true

But it is related to the whole Add to cart chain of events and async process, probably await missing somewhere

gibkigonzo commented 4 years ago

by default this option serverSyncCanRemoveLocalItems is set to false and with that is related all sync process. I think that it is much more complicated then just adding await. It is better to fix this bug after 1.12 release

gibkigonzo commented 3 years ago

I think we should remove serverSyncCanRemoveLocalItems and serverSyncCanModifyLocalItems. Instead of those configs we should check if this is first cart pull (not related with add/remove actions). On that pull we should set forceClientState on false. Which is now always set on true, because serverSyncCanRemoveLocalItems is always false

niro08 commented 1 year ago

So, I faced same Issue and there two ways to fix it: first one is a bit "dirty" fix, in core/modules/cart/store/actions/mergeActions.ts I changed synchronizeServerItem code block inside if (!serverItem) condition, from:

if (!serverItem) {
      Logger.warn('No server item with sku ' + clientItem.sku + ' on stock.', 'cart')()
      diffLog.pushServerParty({ sku: clientItem.sku, status: 'no-item' })
      if (dryRun) return diffLog
      if (forceClientState || !config.cart.serverSyncCanRemoveLocalItems) {
        const updateServerItemDiffLog = await dispatch('updateServerItem', { clientItem, serverItem, updateIds: false })
        return diffLog.merge(updateServerItemDiffLog)
      }
      await dispatch('removeItem', { product: clientItem })
      return diffLog
    }

to:

if (!serverItem) {
      Logger.warn('No server item with sku ' + clientItem.sku + ' on stock.', 'cart')()
      diffLog.pushServerParty({ sku: clientItem.sku, status: 'no-item' })
      const wasSyncedBefore = await StorageManager.get('cart').getItem('synced');
      if (dryRun) return diffLog
      if (forceClientState || !wasSyncedBefore || !config.cart.serverSyncCanRemoveLocalItems) {
        const updateServerItemDiffLog = await dispatch('updateServerItem', { clientItem, serverItem, updateIds: false })
        StorageManager.get('cart').setItem('synced', true)
        return diffLog.merge(updateServerItemDiffLog)
      }

      await dispatch('removeItem', { product: clientItem })
      return diffLog
    }

So actually I just create "cart/synced" localStorage property and checking if it exists before, so updateServerItem is forcing on first calling of the method.

Second fix is much better (as I assume): At the first addToCart attemption create method from core/modules/cart/store/actions/connectActions.ts is calling:

async create ({ dispatch, getters }) {
    const storedItems = getters['getCartItems'] || []
    const cartToken = getters['getCartToken']
    if (storedItems.length && !cartToken) {
      Logger.info('Creating server cart token', 'cart')()
      return dispatch('connect', { guestCart: false })
    }
  }

As you can see connect is calling only if we have cartItems in store already, but not cartToken. I assume this condition can be true only at the first addToCart attemption, so I just added forceClientState parameter - 

  async create ({ dispatch, getters }) {
    const storedItems = getters['getCartItems'] || []
    const cartToken = getters['getCartToken']
    if (storedItems.length && !cartToken) {
      Logger.info('Creating server cart token', 'cart')()
      return dispatch('connect', { guestCart: false, forceClientState: true })
    }
  }

Looks like both solutions works, but I preffer second one, still testing it.

UPD: for second solution connect method (in the same connectActions.ts file) also should be slighty moified for fix in case cart is guest:

  async connect ({ getters, dispatch, commit }, { guestCart = false, forceClientState = false, mergeQty = false }) {
    if (!getters.isCartSyncEnabled) return
    const { result, resultCode } = await CartService.getCartToken(guestCart, forceClientState)

    if (resultCode === 200) {
      Logger.info('Server cart token created.', 'cart', result)()
      commit(types.CART_LOAD_CART_SERVER_TOKEN, result)

      return dispatch('sync', { forceClientState, dryRun: !config.cart.serverMergeByDefault, mergeQty })
    }

    if (resultCode === 401 && getters.bypassCounter < config.queues.maxCartBypassAttempts) {
      Logger.log('Bypassing with guest cart' + getters.bypassCounter, 'cart')()
      commit(types.CART_UPDATE_BYPASS_COUNTER, { counter: 1 })
      Logger.error(result, 'cart')()
      return dispatch('connect', { guestCart: true, forceClientState: true })
    }

    Logger.warn('Cart sync is disabled by the config', 'cart')()
    return createDiffLog()
  }