Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(product-import): Variant reassignment #127

Merged
merged 28 commits into from
Aug 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
eee6f15
feat(productImport): add reassignment functionality and test
lojzatran Oct 24, 2017
d767187
Merge branch 'master' into 116-variant-reassignment
junajan Feb 22, 2018
145c5c8
#2 Update source of reassignment module
junajan Feb 22, 2018
810d2eb
#116 Add dist to test github instalation
junajan Feb 22, 2018
7976357
feat(productImport): change reassignment module to new testing branch…
lojzatran Mar 19, 2018
935a85a
feat(productImport): change reassignment module to testing branch (#116)
lojzatran Mar 19, 2018
31ae197
feat(productImport): return correct value after reassignment (#116)
lojzatran Mar 19, 2018
f99042e
feat(productImport): add statistics of variant reassignment (#116)
lojzatran Apr 11, 2018
2a4f363
feat(productImport): remove redundant message for reassignment summar…
lojzatran Apr 11, 2018
04d36aa
feat(productImport): push dist for testing (#116)
lojzatran Apr 12, 2018
9707945
Merge branch 'master' into 116-variant-reassignment
junajan Jun 6, 2018
63d296a
feat(product-import): Filter out failed products #116 (#121)
junajan Jun 6, 2018
a9fa699
Add debug message
junajan Jun 8, 2018
2b60766
fix(tests): Fix tests - correct expected reassignment statistics
junajan Jun 8, 2018
98120d9
Build latest changes
junajan Jun 8, 2018
906a8c6
chore(import): Reuse reassignment class for all batches
junajan Jun 11, 2018
26b87b7
chore(import): Build dist folder
junajan Jun 11, 2018
9175ca2
fix(tests): Fix tests - reassignment stats
junajan Jun 11, 2018
c501d93
feat(import): Better logging of failed requests from reassignment
junajan Jun 11, 2018
5072559
Change debug logs
junajan Jun 11, 2018
b0d1b04
fix(product-import): Fix reseting statistics
junajan Jun 13, 2018
d011bb0
fix(productImport): Fix variable name
junajan Jun 13, 2018
44d7d7a
feat(productImport): adapt code to newest changes of reassignment mod…
lojzatran Aug 20, 2018
db29954
feat(productImport): remove transpiled files (#116)
lojzatran Aug 20, 2018
995ab72
Merge branch 'master' into 116-variant-reassignment
lojzatran Aug 20, 2018
c0b2b94
feat(productImport): create reassignment instance if reassignment is …
lojzatran Aug 20, 2018
48795df
feat(productImport): add comment to reassignment test
lojzatran Aug 29, 2018
e26df04
feat(productImport): add reassignment option to readme
lojzatran Aug 29, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lib/price-import.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,22 @@ class PriceImport extends ProductImport
Promise.resolve(@_summary)
,{concurrency: 1}

_handleProcessResponse: (res) =>
if res.isFulfilled()
@_handleFulfilledResponse(res)
else if res.isRejected()
error = serializeError res.reason()

@_summary.failed++
if @errorDir
errorFile = path.join(@errorDir, "error-#{@_summary.failed}.json")
fs.outputJsonSync(errorFile, error, {spaces: 2})

if _.isFunction(@errorCallback)
@errorCallback(error, @logger)
else
@logger.error "Error callback has to be a function!"

_handleFulfilledResponse: (r) =>
switch r.value().statusCode
when 201 then @_summary.created++
Expand Down
73 changes: 57 additions & 16 deletions lib/product-import.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ UnknownAttributesFilter = require './unknown-attributes-filter'
CommonUtils = require './common-utils'
EnsureDefaultAttributes = require './ensure-default-attributes'
util = require 'util'
Reassignment = require('commercetools-node-variant-reassignment').default

class ProductImport

Expand Down Expand Up @@ -45,6 +46,12 @@ class ProductImport
# possible values:
# always, publishedOnly, stagedAndPublishedOnly
@publishingStrategy = options.publishingStrategy or false
@variantReassignmentOptions = options.variantReassignmentOptions or {}
if @variantReassignmentOptions.enabled
@reassignmentService = new Reassignment(@client, @logger,
(error) => @_handleErrorResponse(error),
@variantReassignmentOptions.retainExistingData)

@_configErrorHandling(options)
@_resetCache()
@_resetSummary()
Expand Down Expand Up @@ -88,6 +95,9 @@ class ProductImport
productTypeUpdated: 0
errorDir: @errorDir
if @filterUnknownAttributes then @_summary.unknownAttributeNames = []
if @variantReassignmentOptions.enabled
@_summary.variantReassignment = null
@reassignmentService._resetStats()

summaryReport: (filename) ->
message = "Summary: there were #{@_summary.created + @_summary.updated} imported products " +
Expand All @@ -106,6 +116,18 @@ class ProductImport
}
report

_filterOutProductsBySkus: (products, blacklistSkus) ->
return products.filter (product) =>
variants = product.variants.concat(product.masterVariant)
variantSkus = variants.map (v) -> v.sku

# check if at least one SKU from product is in the blacklist
isProductBlacklisted = variantSkus.find (sku) ->
blacklistSkus.indexOf(sku) >= 0

# filter only products which are NOT blacklisted
not isProductBlacklisted

performStream: (chunk, cb) ->
@_processBatches(chunk).then -> cb()

Expand All @@ -132,8 +154,20 @@ class ProductImport
@logger.warn "Filtering out #{filteredProductsLength} product(s) which do not have SKU"
@_summary.productsWithMissingSKU += filteredProductsLength

skus = @_extractUniqueSkus(productsToProcess)
if skus.length then @_getExistingProductsForSkus(skus) else []
if (@variantReassignmentOptions.enabled)
@logger.debug 'execute reassignment process'

@reassignmentService.execute(productsToProcess, @_cache.productType)
.then((res) =>
# if there are products which failed during reassignment, remove them from processing
if(res.badRequestSKUs.length)
@logger.warn(
"Removing #{res.badRequestSKUs} skus from processing due to a reassignment error"
)
productsToProcess = @_filterOutProductsBySkus(productsToProcess, res.badRequestSKUs)
)
.then () =>
@_getExistingProductsForSkus(@_extractUniqueSkus(productsToProcess))
.then (queriedEntries) =>
if @defaultAttributesService
debug 'Ensuring default attributes'
Expand All @@ -146,9 +180,16 @@ class ProductImport
@_createOrUpdate productsToProcess, queriedEntries
.then (results) =>
_.each results, (r) =>
@_handleProcessResponse(r)
if r.isFulfilled()
@_handleFulfilledResponse(r)
else if r.isRejected()
@_handleErrorResponse(r.reason())

Promise.resolve(@_summary)
, { concurrency: 1 } # run 1 batch at a time
.then =>
if @variantReassignmentOptions.enabled
@_summary.variantReassignment = @reassignmentService.statistics

_getWhereQueryLimit: ->
client = @client.productProjections
Expand All @@ -166,6 +207,9 @@ class ProductImport

_getExistingProductsForSkus: (skus) =>
new Promise (resolve, reject) =>
if skus.length == 0
return resolve([])

skuChunks = @commonUtils._separateSkusChunksIntoSmallerChunks(
skus,
@_getWhereQueryLimit()
Expand Down Expand Up @@ -194,21 +238,18 @@ class ProductImport
Error not logged as error limit of #{@errorLimit} has reached.
"

_handleProcessResponse: (res) =>
if res.isFulfilled()
@_handleFulfilledResponse(res)
else if res.isRejected()
error = serializeError res.reason()
_handleErrorResponse: (error) =>
error = serializeError error

@_summary.failed++
if @errorDir
errorFile = path.join(@errorDir, "error-#{@_summary.failed}.json")
fs.outputJsonSync(errorFile, error, {spaces: 2})
@_summary.failed++
if @errorDir
errorFile = path.join(@errorDir, "error-#{@_summary.failed}.json")
fs.outputJsonSync(errorFile, error, {spaces: 2})

if _.isFunction(@errorCallback)
@errorCallback(error, @logger)
else
@logger.error "Error callback has to be a function!"
if _.isFunction(@errorCallback)
@errorCallback(error, @logger)
else
@logger.error "Error callback has to be a function!"

_handleFulfilledResponse: (res) =>
switch res.value().statusCode
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
},
"dependencies": {
"bluebird": "^3.0.0",
"commercetools-node-variant-reassignment": "1.1.0",
"cuid": "^1.3.8",
"debug": "2.6.8",
"fs-extra": "3.0.1",
Expand All @@ -62,6 +63,7 @@
"grunt-shell": "2.1.0",
"jasmine-node": "1.14.5",
"randomstring": "1.1.5",
"sinon": "^5.1.0",
"sphere-coffeelint": "git://github.com/sphereio/sphere-coffeelint.git#master"
},
"keywords": [
Expand Down
2 changes: 2 additions & 0 deletions readme/product-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ Note that this tool can import only products which have at least one variant wit
* the product that should be updated
* the product that is being imported
* an _array_ of update actions that should be ignored
* variantReassignmentOptions
* enabled: when set to `true`, [reassignment module](https://github.com/commercetools/commercetools-node-variant-reassignment) will run before product import.

#### Sample configuration object for cli:

Expand Down
192 changes: 192 additions & 0 deletions test/integration/product-import.spec.coffee
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
os = require 'os'
path = require 'path'
sinon = require 'sinon'
debug = require('debug')('spec:it:sphere-product-import')
_ = require 'underscore'
_.mixin require 'underscore-mixins'
Expand Down Expand Up @@ -384,3 +385,194 @@ describe 'Product Importer integration tests', ->
expect(product.masterVariant.attributes.length).toBe(900)
expect(product.variants[0].attributes.length).toBe(635)
done()

describe 'product variant reassignment', ->
beforeEach (done) ->
reassignmentConfig = _.extend config, { variantReassignmentOptions: { enabled: true } }
@import = new ProductImport logger, reassignmentConfig
done()

# Reassignment before:
# existing product "foo": sku1, sku2, sku3
# existing product "no-category": sku4, sku5, sku6
# new product draft "reassigned-product": sku4, sku3
# ---
# Reassignment before:
# "no-category" will be reassigned because of matched by master variant "reassinged-product": sku4, sku3
# 1 new anonymized product "no-category-randomSlug": sku5, sku6
# existing product "foo": sku1, sku2
it 'should reassign product variants', (done) ->
productDraft1 = createProduct()[0]
productDraft1.productType.id = @productType.id
productDraft1.categories[0].id = @category.id
productDraft2 = createProduct()[1]
productDraft2.productType.id = @productType.id
productDraft2.categories[0].id = @category.id
Promise.all([
ensureResource(@client.products, "masterData(staged(slug(en=\"#{productDraft1.slug.en}\")))", productDraft1)
ensureResource(@client.products, "masterData(staged(slug(en=\"#{productDraft2.slug.en}\")))", productDraft2)
])
.then () =>
productDraftChunk = {
productType:
typeId: 'product-type'
id: @productType.name
"name": {
"en": "reassigned-product"
},
"categories": [
{
"typeId": "category",
"id": "test-category"
}
],
"categoryOrderHints": {},
"slug": {
"en": "reassigned-product"
},
"masterVariant": {
"sku": "sku4",
"prices": [],
"images": [],
"attributes": [],
"assets": []
},
"variants": [
{
"sku": "sku3",
"prices": [
{
"value": {
"currencyCode": "GBP",
"centAmount": 9
},
"id": "e9748681-e42f-45c4-b07f-48b92c6e910a"
}
],
"images": [],
"attributes": [],
"assets": []
}
],
"searchKeywords": {}
}
@import.performStream([productDraftChunk], Promise.resolve)
lojzatran marked this conversation as resolved.
Show resolved Hide resolved
.then =>
expect(@import._summary.variantReassignment.anonymized).toEqual(1)
expect(@import._summary.variantReassignment.productTypeChanged).toEqual(0)
expect(@import._summary.variantReassignment.processed).toEqual(1)
expect(@import._summary.variantReassignment.succeeded).toEqual(1)
expect(@import._summary.variantReassignment.transactionRetries).toEqual(0)
expect(@import._summary.variantReassignment.badRequestErrors).toEqual(0)
expect(@import._summary.variantReassignment.processedSkus).toEqual([ 'sku4', 'sku3' ])
expect(@import._summary.variantReassignment.badRequestSKUs).toEqual([])
expect(@import._summary.variantReassignment.anonymizedSlug[0].indexOf('no-category')).not.toEqual(-1)

@fetchProducts(@productType.id)
.then ({ body: { results } }) =>
expect(results.length).toEqual(3)
reassignedProductNoCategory = _.find results, (p) => p.name.en == 'reassigned-product'
expect(reassignedProductNoCategory.slug.en).toEqual('reassigned-product')
expect(reassignedProductNoCategory.masterVariant.sku).toEqual('sku4')
expect(reassignedProductNoCategory.variants.length).toEqual(1)
expect(reassignedProductNoCategory.variants[0].sku).toEqual('sku3')

fooProduct = _.find results, (p) => p.name.en == 'foo'
expect(fooProduct.slug.en).toEqual('foo')
expect(fooProduct.masterVariant.sku).toEqual('sku1')
expect(fooProduct.variants.length).toEqual(1)
expect(fooProduct.variants[0].sku).toEqual('sku2')

anonymizedProduct = _.find results, (p) => p.name.en == 'no-category'
expect(anonymizedProduct.slug.ctsd).toBeDefined()
expect(anonymizedProduct.variants.length).toEqual(1)
skus = anonymizedProduct.variants.concat(anonymizedProduct.masterVariant)
.map((v) => v.sku)
expect(skus).toContain('sku5')
expect(skus).toContain('sku6')
done()
.catch (err) =>
done(_.prettify err)

it 'should handle product reassignment error', (done) ->
stubCreateOrUpdate = sinon.stub(@import, '_createOrUpdate')
.callThrough()

productDraft1 = createProduct()[0]
productDraft1.productType.id = @productType.id
productDraft1.categories[0].id = @category.id
productDraft2 = createProduct()[1]
productDraft2.productType.id = @productType.id
productDraft2.categories[0].id = @category.id
Promise.all([
ensureResource(@client.products, "masterData(staged(slug(en=\"#{productDraft1.slug.en}\")))", productDraft1)
ensureResource(@client.products, "masterData(staged(slug(en=\"#{productDraft2.slug.en}\")))", productDraft2)
])
.then () =>
productDraftChunk = {
productType:
typeId: 'product-type'
id: @productType.name
"name": {
"en": "reassigned-product"
},
"categories": [
{
"typeId": "category",
"id": "test-category"
}
],
"categoryOrderHints": {},
"slug": {
"en": "reassigned-product"
},
"masterVariant": {
"sku": "sku4",
"prices": [],
"images": [],
"attributes": [],
"assets": []
},
"variants": [
{
"sku": "sku3",
"prices": [
{
"value": {
"currencyCode": "GBP",
"centAmount": 'INVALID PRICE'
},
"id": "e9748681-e42f-45c4-b07f-48b92c6e910a"
}
],
"images": [],
"attributes": [],
"assets": []
}
],
"searchKeywords": {}
}
@import.performStream([productDraftChunk], Promise.resolve)
.then =>
expect(stubCreateOrUpdate.callCount).toEqual(1)
productsToProcess = stubCreateOrUpdate.firstCall.args[0]
queriedEntries = stubCreateOrUpdate.firstCall.args[1]

expect(productsToProcess.length).toBe(0)
expect(queriedEntries.length).toBe(0)
expect(@import._summary.failed).toBe(1)

expect(@import._summary.variantReassignment).toEqual({
anonymized: 0,
productTypeChanged: 0,
processed: 1,
succeeded: 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to be sure, are these reassignment stats correctly named? (proper keys - as we changed them in reassignment )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked, and it is correct naming.

transactionRetries: 0,
badRequestErrors: 1,
processedSkus: ['sku4', 'sku3']
badRequestSKUs: ['sku4', 'sku3']
anonymizedSlug: []
})
done()
.catch (err) =>
done(_.prettify err)