vue-storefront-1 icon indicating copy to clipboard operation
vue-storefront-1 copied to clipboard

Add to cart broken for new users with server sync on

Open AgarvaiN opened this issue 5 years ago • 3 comments

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?

  • [ ] YES
  • [x] NO

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

Pick one option.

  • [ ] This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • [ ] This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • [x] This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

  • Browser: Chrome 81.0.4044.122 and others
  • OS: All (Mac OS, Windows 10, Ubuntu Server, Linux Mint)
  • Node: 10-13 (Currently 13.13.0)
  • Code Version: Master - 1.11.3

Additional information

Important setting "serverSyncCanRemoveLocalItems": true

  • I think this is the one that breaks it

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

AgarvaiN avatar Apr 28 '20 09:04 AgarvaiN

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 avatar May 22 '20 11:05 gibkigonzo

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

gibkigonzo avatar Jan 15 '21 08:01 gibkigonzo

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()
  }

niro08 avatar Oct 24 '22 16:10 niro08