Merge 6c25e4a into ae22724

commercetools · Mar 6, 2020 · a345379 · a345379
2 parents ae22724 + 6c25e4a
commit a345379
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 46 deletions.
diff --git a/src/coffee/sync/utils/product.coffee b/src/coffee/sync/utils/product.coffee
@@ -124,14 +124,7 @@ class ProductUtils extends BaseUtils
       action =
         action: 'changeMasterVariant'
 
-      if newMasterVariant.sku
-        action.sku = newMasterVariant.sku
-      else if newMasterVariant.id
-        action.variantId = newMasterVariant.id
-      else
-        throw new Error(
-          'ProductSync needs at least one of "id" or "sku" to generate changeMasterVariant update action'
-        )
+      @_setSkuOrVariantIdToUpdateAction(newMasterVariant, action)
       action
 
   buildRemoveVariantActions: (newVariants, oldVariants) ->
@@ -402,8 +395,8 @@ class ProductUtils extends BaseUtils
       delete price._MATCH_CRITERIA
       action =
         action: 'addPrice'
-        variantId: old_variant.id
         price: price
+      @_setSkuOrVariantIdToUpdateAction(old_variant, action)
     action
 
   buildVariantImagesAction: (images, old_variant, new_variant) ->
@@ -432,13 +425,12 @@ class ProductUtils extends BaseUtils
 
           else if _.has(image, 'label') and
           (image.label.length == 1 or image.label.length == 2)
-
-            actions.push(
+            action =
               action: 'changeImageLabel'
-              variantId: old_variant.id
               imageUrl: old_variant.images[key].url
               label: new_variant.images[key].label
-            )
+            @_setSkuOrVariantIdToUpdateAction(old_variant, action)
+            actions.push(action)
 
       else if REGEX_UNDERSCORE_NUMBER.test key
         index = key.substring(1)
@@ -452,16 +444,16 @@ class ProductUtils extends BaseUtils
       delete image._MATCH_CRITERIA
       action =
         action: 'addExternalImage'
-        variantId: variant.id
         image: image
+      @_setSkuOrVariantIdToUpdateAction(variant, action)
     action
 
   _buildRemoveImageAction: (variant, image) ->
     if image
       action =
         action: 'removeImage'
-        variantId: variant.id
         imageUrl: image.url
+      @_setSkuOrVariantIdToUpdateAction(variant, action)
     action
 
   _isExistingAttribute: (oldAttribute, newAttribute) ->
@@ -477,8 +469,10 @@ class ProductUtils extends BaseUtils
     if newAttribute
       action =
         action: 'setAttribute'
-        variantId: old_variant.id
         name: newAttribute.name
+
+      @_setSkuOrVariantIdToUpdateAction(old_variant, action)
+
       oldAttribute = _.find old_variant.attributes, (attrib) ->
         attrib.name is newAttribute.name
 
@@ -526,15 +520,16 @@ class ProductUtils extends BaseUtils
               action.value = text
     action
 
-  _buildNewSetAttributeAction: (id, el, sameForAllAttributeNames) ->
+  _buildNewSetAttributeAction: (oldVariant, el, sameForAllAttributeNames) ->
     attributeName = el?.name
     return unless attributeName
     action =
       action: "setAttribute"
-      variantId: id
       name: attributeName
       value: el.value
 
+    @_setSkuOrVariantIdToUpdateAction(oldVariant, action)
+
     if _.isArray(action.value)
       _.each action.value, (v) ->
         delete v._MATCH_CRITERIA unless _.isString(v)
@@ -544,15 +539,22 @@ class ProductUtils extends BaseUtils
       delete action.variantId
     action
 
+  _setSkuOrVariantIdToUpdateAction: (variant, action) ->
+    if variant.sku
+      action.sku = variant.sku
+    else if variant.id
+      action.variantId = variant.id
+    else
+      throw new Error("ProductSync needs at least one of \"id\" or \"sku\" to generate \"#{action.action}\" update action")
+
   buildVariantAttributesActions: (attributes, old_variant, new_variant, sameForAllAttributeNames) ->
     actions = []
     if attributes
       _.each attributes, (value, key) =>
         if REGEX_NUMBER.test key
           if _.isArray value
             deltaValue = @getDeltaValue(value)
-            id = old_variant.id
-            setAction = @_buildNewSetAttributeAction(id, deltaValue, sameForAllAttributeNames)
+            setAction = @_buildNewSetAttributeAction(old_variant, deltaValue, sameForAllAttributeNames)
             actions.push setAction if setAction
           else
             # key is index of attribute
@@ -572,8 +574,7 @@ class ProductUtils extends BaseUtils
                 deltaValue = { name: value[0].name }
               else
                 deltaValue = undefined
-            id = old_variant.id
-            setAction = @_buildNewSetAttributeAction(id, deltaValue, sameForAllAttributeNames)
+            setAction = @_buildNewSetAttributeAction(old_variant, deltaValue, sameForAllAttributeNames)
             actions.push setAction if setAction
           else
             index = key.substring(1)
@@ -593,8 +594,9 @@ class ProductUtils extends BaseUtils
     if _.has variantDiff, 'key'
       action =
         action: 'setProductVariantKey'
-        variantId: old_variant.id
         key: @getDeltaValue(variantDiff.key)
+      @_setSkuOrVariantIdToUpdateAction(old_variant, action)
+      action
 
 module.exports = ProductUtils
 

diff --git a/src/spec/client/services/base.spec.coffee b/src/spec/client/services/base.spec.coffee
@@ -149,7 +149,7 @@ describe 'Service', ->
       _.each ['30s', '15m', '12h', '7d', '2w'], (type) ->
         it "should allow to query for last #{type}", ->
           @service.last(type)
-          expect(@service._params.query.where[0]).toMatch /lastModifiedAt%20%3E%20%22201\d-\d\d-\d\dT\d\d%3A\d\d%3A\d\d.\d\d\dZ%22/
+          expect(@service._params.query.where[0]).toMatch /lastModifiedAt%20%3E%20%2220\d\d-\d\d-\d\dT\d\d%3A\d\d%3A\d\d.\d\d\dZ%22/
 
       it 'should throw an exception when the period for last can not be parsed', ->
         expect(=> @service.last('30')).toThrow new Error "Cannot parse period '30'"

diff --git a/src/spec/sync/product-sync-sku-match.spec.coffee b/src/spec/sync/product-sync-sku-match.spec.coffee
@@ -71,7 +71,7 @@ describe 'ProductUtils SKU based matching', ->
       { action: 'addVariant', sku: 'vN', attributes: [{name: 'attribN', value: 'valN'}] }
     ]
     compareAttributeActions @sync, @oldProduct, @newProduct, [
-      { action: 'setAttribute', variantId: 9, name: 'attrib', value: 'CHANGED' }
+      { action: 'setAttribute', sku: 'v2', name: 'attrib', value: 'CHANGED' }
     ]
 
   it 'should work when the order of variant has changed', ->
@@ -91,8 +91,8 @@ describe 'ProductUtils SKU based matching', ->
     compareVariantActions @sync, @oldProduct, @newProduct,  []
 
     compareAttributeActions @sync, @oldProduct, @newProduct, [
-      { action: 'setAttribute', variantId: 5, name: 'attrib5', value: 'CHANGED5' }
-      { action: 'setAttribute', variantId: 2, name: 'attrib2', value: 'CHANGED2' }
+      { action: 'setAttribute', sku: 'v5', name: 'attrib5', value: 'CHANGED5' }
+      { action: 'setAttribute', sku: 'v2', name: 'attrib2', value: 'CHANGED2' }
     ]
 
   it 'should work in combination with variant additions and removes', ->
@@ -115,8 +115,8 @@ describe 'ProductUtils SKU based matching', ->
     ]
 
     compareAttributeActions @sync, @oldProduct, @newProduct, [
-      { action: 'setAttribute', variantId: 3, name: 'attrib3', value: 'CHANGED3' }
-      { action: 'setAttribute', variantId: 5, name: 'attrib5', value: 'CHANGED5' }
+      { action: 'setAttribute', sku: 'v3', name: 'attrib3', value: 'CHANGED3' }
+      { action: 'setAttribute', sku: 'v5', name: 'attrib5', value: 'CHANGED5' }
     ]
 
   it 'should work when master variant was switched with another variant', ->
@@ -135,6 +135,6 @@ describe 'ProductUtils SKU based matching', ->
       { action : 'changeMasterVariant', sku : 'v3' }
     ]
     compareAttributeActions @sync, @oldProduct, @newProduct, [
-      { action: 'setAttribute', variantId: 3, name: 'attrib3', value: undefined } # remove from master
-      { action: 'setAttribute', variantId: 1, name: 'attrib3', value: 'CHANGED3' } # set on variant v1
+      { action: 'setAttribute', sku: 'v3', name: 'attrib3', value: undefined } # remove from master
+      { action: 'setAttribute', sku: 'v1', name: 'attrib3', value: 'CHANGED3' } # set on variant v1
     ]
diff --git a/src/spec/sync/product-sync.spec.coffee b/src/spec/sync/product-sync.spec.coffee
@@ -495,8 +495,8 @@ describe 'ProductSync', ->
 
         update = @sync._doMapVariantActions newProduct, oldProduct
         expect(update.variantUpdateActions).toEqual [
-          {action: 'addPrice', variantId: 2, price: {value: {currencyCode: 'EUR', centAmount: 5}, customerGroup: {id: '987', typeId: 'customer-group'}}}
-          {action: 'addPrice', variantId: 2, price: {value: {currencyCode: 'EUR', centAmount: 20}, country: 'DE'}}
+          {action: 'addPrice', sku: 'foo', price: {value: {currencyCode: 'EUR', centAmount: 5}, customerGroup: {id: '987', typeId: 'customer-group'}}}
+          {action: 'addPrice', sku: 'foo', price: {value: {currencyCode: 'EUR', centAmount: 20}, country: 'DE'}}
         ]
 
       it 'should build change price actions', ->
@@ -878,10 +878,10 @@ describe 'ProductSync', ->
         variantActionTypes = @sync._doMapVariantActions(NEW, OLD)
 
         expected_update = [
-          { action: 'setProductVariantKey', variantId: 1, key: 'newKey' }
-          { action: 'setProductVariantKey', variantId: 2, key: 'newVariantKey' }
-          { action: 'setProductVariantKey', variantId: 3, key: undefined }
-          { action: 'setProductVariantKey', variantId: 4, key: 'newVariantKey3' }
+          { action: 'setProductVariantKey', sku: 'masterSku', key: 'newKey' }
+          { action: 'setProductVariantKey', sku: 'variantSku', key: 'newVariantKey' }
+          { action: 'setProductVariantKey', sku: 'variantSku2', key: undefined }
+          { action: 'setProductVariantKey', sku: 'variantSku3', key: 'newVariantKey3' }
         ]
         expect(variantActionTypes.variantUpdateActions).toEqual expected_update
 
@@ -1139,8 +1139,8 @@ describe 'ProductSync', ->
         update = @sync._doMapVariantActions newProduct, oldProduct
         expected_update =
           [
-            { action: 'setAttribute', variantId: 1, name: 'testAttribute1', value: false },
-            { action: 'setAttribute', variantId: 1, name: 'testAttribute2' }
+            { action: 'setAttribute', sku: 'test_sku_1', name: 'testAttribute1', value: false },
+            { action: 'setAttribute', sku: 'test_sku_1', name: 'testAttribute2' }
           ]
         # should not modify original old project
         expect(oldProductClone.masterVariant.attributes).toEqual oldProduct.masterVariant.attributes
@@ -1374,9 +1374,9 @@ describe 'ProductSync', ->
       update = @sync.buildActions(newProduct, oldProduct).getUpdatePayload()
       expected_update =
         actions: [
-          { action: 'setAttribute', variantId: 1, name: 'foo', value: 'new value' }
-          { action: 'setAttribute', variantId: 2, name: 'foo', value: 'another value' }
-          { action: 'setAttribute', variantId: 3, name: 'foo', value: 'yet another' }
+          { action: 'setAttribute', sku: 'v1', name: 'foo', value: 'new value' }
+          { action: 'setAttribute', sku: 'v2', name: 'foo', value: 'another value' }
+          { action: 'setAttribute', sku: 'v3', name: 'foo', value: 'yet another' }
           { action: 'addVariant', id: 4, sku: 'v4', attributes: [{ name: 'foo', value: 'i dont care' }] }
         ]
         version: oldProduct.version

diff --git a/src/spec/sync/utils/product.spec.coffee b/src/spec/sync/utils/product.spec.coffee
@@ -499,7 +499,7 @@ describe 'ProductUtils', ->
         key: 'oldVar'
 
       expect( () => @utils.buildChangeMasterVariantAction(newMasterVariant, oldMasterVariant))
-        .toThrow new Error('ProductSync needs at least one of "id" or "sku" to generate changeMasterVariant update action')
+        .toThrow new Error('ProductSync needs at least one of "id" or "sku" to generate "changeMasterVariant" update action')
 
   describe ':: buildVariantBaseAction', ->
 
@@ -818,9 +818,9 @@ describe 'ProductUtils', ->
       update = @utils.buildVariantAttributesActions diff.attributes, oldVariant, newVariant
       expected_update =
         [
-          { action: 'setAttribute', variantId: 1, name: 'images', value: '//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-1.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-2.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-4.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-5.jpg' },
-          { action: 'setAttribute', variantId: 1, name: 'textAttribute', value: '[{"textAttributeValue":{"fr-CH":"","de-CH":"","it-CH":"","de-DE":"<p><strong>Some random text to make this longer than the value that was in jsondiffpatch.textDiff.minLength = 300. This should be now a correctly formatted JSON. However, after jsondiffpatch, it will be changed into a different string”</p>","en-GB":"","es-ES":"","fr-FR":""}}]' },
-          { action: 'setAttribute', variantId: 1, name: 'localized_images', value: { en: '//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-1.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-2.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-4.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-5.jpg' } }
+          { action: 'setAttribute', sku: 'HARJPUL101601202', name: 'images', value: '//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-1.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-2.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-4.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-5.jpg' },
+          { action: 'setAttribute', sku: 'HARJPUL101601202', name: 'textAttribute', value: '[{"textAttributeValue":{"fr-CH":"","de-CH":"","it-CH":"","de-DE":"<p><strong>Some random text to make this longer than the value that was in jsondiffpatch.textDiff.minLength = 300. This should be now a correctly formatted JSON. However, after jsondiffpatch, it will be changed into a different string”</p>","en-GB":"","es-ES":"","fr-FR":""}}]' },
+          { action: 'setAttribute', sku: 'HARJPUL101601202', name: 'localized_images', value: { en: '//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-1.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-2.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-4.jpg;//dceroyf7rfc0x.cloudfront.net/product/images/390x520/a/arj/po/HARJPUL101601-5.jpg' } }
         ]
       expect(update).toEqual expected_update