bugfix - #4132, #4133, #4136, #4138, #4139, #4140

CHANGELOG.md

@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 Remove and add coupon when user login Remove 'NA' as default company. Show qty in microcart for all types of product.
 Remove preload font - it gives good performance, but vue-meta refresh page, because there is script onload. - @gibkigonzo (pr#4128)
 - Keep old category before route is resolved - @gibkigonzo (pr#4124)
+- Added comments in 'productsEqual' and change logic for different types of products. Remove login user after order in Checkout. Allow changing qty for 'group' and 'bundle'.products - @gibkigonzo (pr#4144)
 
 ## [1.11.1] - 2020.02.05
 

core/modules/cart/helpers/productChecksum.ts

@@ -27,7 +27,7 @@ export const getProductOptions = (product, optionsName) => {
 
 const getDataToHash = (product: CartItem): any => {
   if (!product.product_option) {
-    return product.sku ? product.sku : null
+    return null
   }
 
   const supportedProductOptions = ['bundle_options', 'custom_options', 'configurable_item_options']

core/modules/cart/helpers/productsEquals.ts

@@ -6,7 +6,7 @@ type ProductEqualCheckFn = (product1: CartItem, product2: CartItem) => boolean
 // 'id' check
 const getServerItemId = (product: CartItem): string | number =>
   product.server_item_id || product.item_id
-const isServerIdsEquals = (product1, product2) => {
+const isServerIdsEquals = (product1: CartItem, product2: CartItem): boolean => {
   const product1ItemId = getServerItemId(product1)
   const product2ItemId = getServerItemId(product2)
 
@@ -70,18 +70,26 @@ const productsEquals = (product1: CartItem, product2: CartItem): boolean => {
   const check = makeCheck.bind(null, product1, product2)
 
   if (getProductOptions(product1, 'bundle_options').length || getProductOptions(product2, 'bundle_options').length) {
+    // bundle options skus are merged into one sku so we can't rely on 'sku'
+    // by default we want to check server_item_id ('id'), we can also use 'checksum'
     return check(['id', 'checksum'])
   }
 
   if (getProductOptions(product1, 'custom_options').length || getProductOptions(product2, 'custom_options').length) {
+    // in admin panel we can add different sku for specific custom option so we can't rely on 'sku'
+    // by default we want to check server_item_id ('id'), we can also use 'checksum'
     return check(['id', 'checksum'])
   }
 
   if (getProductOptions(product1, 'configurable_item_options').length || getProductOptions(product2, 'configurable_item_options').length) {
-    return check(['checksum', 'sku'])
+    // 'sku' should be uniq for configurable products
+    // we can't check 'id' because it is the same when user edit product in microcart, so it can give wrong result
+    return check(['sku'])
   }
 
-  return check(['id', 'checksum', 'sku'])
+  // by default we want to check if server_item_id is equal and check sku as fallback
+  // this is for 'simple' and 'group' products
+  return check(['id', 'sku'])
 }
 
 export default productsEquals

core/modules/cart/store/actions/itemActions.ts

@@ -11,7 +11,7 @@ import {
 import { cartHooksExecutors } from './../../hooks'
 
 const itemActions = {
-  configureItem (context, { product, configuration }) {
+  async configureItem (context, { product, configuration }) {
     const { commit, dispatch, getters } = context
     const variant = configureProductAsync(context, {
       product,
@@ -29,7 +29,7 @@ const itemActions = {
     commit(types.CART_UPD_ITEM_PROPS, { product: { ...product, ...variant } })
 
     if (getters.isCartSyncEnabled && product.server_item_id) {
-      dispatch('sync', { forceClientState: true })
+      await dispatch('sync', { forceClientState: true })
     }
   },
   updateItem ({ commit }, { product }) {

core/modules/cart/test/unit/helpers/productEquals.spec.ts

@@ -37,6 +37,27 @@ const createBundleProduct = ({ id, sku, type_id, options }): CartItem => ({
   }
 } as any as CartItem)
 
+const createCustomOptions = (options) => {
+  if (!options) {
+    return []
+  }
+
+  return options.map((option, index) => ({
+    option_id: index + 1,
+    option_value: option
+  }))
+}
+
+const createCustomOptionsProduct = ({ id, sku, options }): CartItem => ({
+  sku,
+  server_item_id: id,
+  product_option: {
+    extension_attributes: {
+      custom_options: createCustomOptions(options)
+    }
+  }
+} as any as CartItem)
+
 const createConfigurableProduct = ({ id, sku }): CartItem => ({
   sku,
   type_id: 'configurable',
@@ -58,31 +79,58 @@ const createConfigurableProduct = ({ id, sku }): CartItem => ({
 } as any as CartItem)
 
 describe('Cart productEquals', () => {
-  it('returns true because bundle products have the same options selected', async () => {
-    const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
-    const product2 = createBundleProduct({ id: 2, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
+  describe('bundle product', () => {
+    it('returns true because products have the same options selected', async () => {
+      const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
+      const product2 = createBundleProduct({ id: 2, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
+
+      expect(productsEquals(product1, product2)).toBeTruthy()
+    });
+
+    it('returns true because products have the same server id', async () => {
+      const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: null })
+      const product2 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'none', options: [2, 4, 5, 8] })
+
+      expect(productsEquals(product1, product2)).toBeTruthy()
+    });
+
+    it('returns false because products have not the same options selected', async () => {
+      const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: [2, 2, 5, 8] })
+      const product2 = createBundleProduct({ id: 2, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
+
+      expect(productsEquals(product1, product2)).toBeFalsy()
+    });
+  })
+
+  describe('custom options product', () => {
+    it('returns true because products have the same options selected', async () => {
+      const product1 = createCustomOptionsProduct({ id: 1, sku: 'WG-001', options: [2, 4, 5, 8] })
+      const product2 = createCustomOptionsProduct({ id: 2, sku: 'WG-001', options: [2, 4, 5, 8] })
 
-    expect(productsEquals(product1, product2)).toBeTruthy()
-  });
+      expect(productsEquals(product1, product2)).toBeTruthy()
+    });
 
-  it('returns false because bundle products have not the same options selected', async () => {
-    const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: [2, 2, 5, 8] })
-    const product2 = createBundleProduct({ id: 2, sku: 'WG-001', type_id: 'bundle', options: [2, 4, 5, 8] })
+    it('returns true because products have the same server id', async () => {
+      const product1 = createCustomOptionsProduct({ id: 1, sku: 'WG-001', options: null })
+      const product2 = createCustomOptionsProduct({ id: 1, sku: 'WG-001', options: [2, 4, 5, 8] })
 
-    expect(productsEquals(product1, product2)).toBeFalsy()
-  });
+      expect(productsEquals(product1, product2)).toBeTruthy()
+    });
 
-  it('returns true because bundle products have the same server id', async () => {
-    const product1 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'bundle', options: null })
-    const product2 = createBundleProduct({ id: 1, sku: 'WG-001', type_id: 'none', options: [2, 4, 5, 8] })
+    it('returns false because products have not the same options selected', async () => {
+      const product1 = createCustomOptionsProduct({ id: 1, sku: 'WG-001', options: [2, 2, 5, 8] })
+      const product2 = createCustomOptionsProduct({ id: 2, sku: 'WG-001', options: [2, 4, 5, 8] })
 
-    expect(productsEquals(product1, product2)).toBeTruthy()
-  });
+      expect(productsEquals(product1, product2)).toBeFalsy()
+    });
+  })
 
-  it('returns true because configurable products have the same sku', async () => {
-    const product1 = createConfigurableProduct({ id: 1, sku: 'WG-001' })
-    const product2 = createConfigurableProduct({ id: 2, sku: 'WG-001' })
+  describe('configurable product', () => {
+    it('returns true because products have the same sku', async () => {
+      const product1 = createConfigurableProduct({ id: 1, sku: 'WG-001' })
+      const product2 = createConfigurableProduct({ id: 2, sku: 'WG-001' })
 
-    expect(productsEquals(product1, product2)).toBeTruthy()
-  });
+      expect(productsEquals(product1, product2)).toBeTruthy()
+    });
+  })
 });

core/modules/checkout/components/OrderReview.ts

@@ -55,8 +55,8 @@ export const OrderReview = {
           }]
         })
 
-        this.$bus.$emit('notification-progress-stop')
         if (result.code !== 200) {
+          this.$bus.$emit('notification-progress-stop')
           this.onFailure(result)
           // If error includes a word 'password', emit event that eventually focuses on a corresponding field
           if (result.result.includes(i18n.t('password'))) {
@@ -72,6 +72,7 @@ export const OrderReview = {
             username: this.getPersonalDetails.emailAddress,
             password: this.getPersonalDetails.password
           })
+          this.$bus.$emit('notification-progress-stop')
           this.$bus.$emit('checkout-before-placeOrder', result.result.id)
           this.onSuccess()
         }

core/modules/checkout/components/Payment.ts

@@ -244,7 +244,9 @@ export const Payment = {
       }
 
       // Let anyone listening know that we've changed payment method, usually a payment extension.
-      this.$bus.$emit('checkout-payment-method-changed', this.payment.paymentMethod)
+      if (this.payment.paymentMethod) {
+        this.$bus.$emit('checkout-payment-method-changed', this.payment.paymentMethod)
+      }
     },
     changeCountry () {
       this.$store.dispatch('checkout/updatePaymentDetails', { country: this.payment.country })

core/modules/checkout/test/unit/components/Payment.spec.ts

@@ -587,8 +587,11 @@ describe('Payment', () => {
       expect((wrapper.vm as any).notInMethods('invalid payment method')).toBe(true);
     });
 
-    it('changePaymentMethod method should emit an event', () => {
+    it('changePaymentMethod method should emit an event when there is paymentMethod', () => {
       mockMethods['changePaymentMethod'].mockRestore();
+      mockStore.modules.checkout.getters.getPaymentDetails.mockImplementation(() => ({
+        paymentMethod: 'payment method'
+      }));
 
       const wrapper = mountMixinWithStore(Payment, mockStore, mockMountingOptions);
       (wrapper.vm as any).changePaymentMethod();

core/pages/Checkout.js

@@ -144,9 +144,6 @@ export default {
     },
     async onAfterPlaceOrder (payload) {
       this.confirmation = payload.confirmation
-      if (this.$store.state.checkout.personalDetails.createAccount) {
-        await this.$store.dispatch('user/login', { username: this.$store.state.checkout.personalDetails.emailAddress, password: this.$store.state.checkout.personalDetails.password })
-      }
       this.$store.dispatch('checkout/setThankYouPage', true)
       this.$store.dispatch('user/getOrdersHistory', { refresh: true, useCache: true })
       Logger.debug(payload.order)()

src/themes/default/components/core/ProductQuantity.vue

@@ -65,10 +65,17 @@ export default {
       return onlineHelper.isOnline
     },
     max () {
-      return this.isOnline ? this.maxQuantity : null
+      if (!this.isOnline || !this.isSimpleOrConfigurable) {
+        return null
+      }
+
+      return this.maxQuantity
     },
     disabled () {
-      return this.isOnline ? !this.maxQuantity && this.checkMaxQuantity : false
+      if (!this.isOnline) {
+        return false
+      }
+      return !this.maxQuantity && this.checkMaxQuantity && this.isSimpleOrConfigurable
     },
     name () {
       if (this.isSimpleOrConfigurable && !this.loading && this.showQuantity) {

src/themes/default/components/core/ProductTile.vue

@@ -206,7 +206,7 @@ $color-white: color(white);
   &__thumb {
     padding-bottom: calc(143.88% / (164.5 / 100));
     @media screen and (min-width: 768px) {
-      padding-bottom: calc(300% / (276.5 / 100));
+      padding-bottom: calc(300% / (276.5 / 115));
     }
     opacity: 0.8;
     will-change: opacity, transform;

src/themes/default/components/core/blocks/Checkout/CartSummary.vue

@@ -4,7 +4,7 @@
       <h3 class="cl-accent ml30 mt50 summary-title">
         {{ $t('Order Summary') }}
       </h3>
-      <product v-for="product in productsInCart" :key="product.sku" :product="product" />
+      <product v-for="product in productsInCart" :key="product.server_item_id || product.id" :product="product" />
       <div v-if="productsInCart && productsInCart.length" class="checkout bg-cl-secondary pt10 serif cl-accent">
         <div v-for="(segment, index) in totals" :key="index" class="row pt15 pb20 pl30 pr55 " v-if="segment.code !== 'grand_total'">
           <div class="col-xs cl-accent">

src/themes/default/components/core/blocks/Microcart/EditMode.vue

@@ -30,7 +30,6 @@ export default {
         color: { id: color.id, attribute_code: color.type, label: color.label }
       }
       this.product.qty = this.getEditingQty
-      this.product.checksum = null
       this.$store.dispatch('cart/configureItem', { product: this.product, configuration })
     },
     getEditedProduct (filter = {}) {

src/themes/default/components/core/blocks/Microcart/Microcart.vue

@@ -49,7 +49,7 @@
       {{ $t('to find something beautiful for You!') }}
     </div>
     <ul v-if="productsInCart.length" class="bg-cl-primary m0 px40 pb40 products">
-      <product v-for="product in productsInCart" :key="product.checksum" :product="product" />
+      <product v-for="product in productsInCart" :key="product.server_item_id || product.id" :product="product" />
     </ul>
     <div v-if="productsInCart.length" class="summary px40 cl-accent serif">
       <h3 class="m0 pt40 mb30 weight-400 summary-heading">