From b6161d24043b8b910320475b8616b7e29a96f6cd Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Mon, 12 Sep 2022 15:53:45 +0200 Subject: [PATCH] fix(medusa): Export/import fixes including export fields that contains new line char (#2150) --- .changeset/small-walls-poke.md | 5 + .../__tests__/batch-jobs/product/export.js | 104 +++++++ .../__tests__/batch-jobs/product/import.js | 283 +++++++++--------- .../batch-jobs/product/product-import-ss.csv | 2 +- .../batch-jobs/product/product-import.csv | 4 +- .../api/src/services/local-file-service.js | 87 +++--- .../routes/admin/batch/create-batch-job.ts | 7 +- packages/medusa/src/services/batch-job.ts | 6 +- .../medusa/src/services/product-variant.ts | 2 +- .../__fixtures__/product-export-data.ts | 9 +- .../product/__snapshots__/export.ts.snap | 8 +- .../__tests__/batch-jobs/product/export.ts | 168 ++++++----- .../__tests__/batch-jobs/product/import.ts | 5 +- .../strategies/batch-jobs/product/export.ts | 17 +- .../strategies/batch-jobs/product/import.ts | 209 ++++++++----- .../strategies/batch-jobs/product/index.ts | 4 +- .../strategies/batch-jobs/product/types.ts | 9 + .../strategies/batch-jobs/product/utils.ts | 11 +- packages/medusa/src/subscribers/batch-job.ts | 59 ++-- .../csv-cell-content-formatter.spec.ts | 43 +++ .../src/utils/csv-cell-content-formatter.ts | 20 ++ packages/medusa/src/utils/index.ts | 1 + 22 files changed, 699 insertions(+), 364 deletions(-) create mode 100644 .changeset/small-walls-poke.md create mode 100644 packages/medusa/src/utils/__tests__/csv-cell-content-formatter.spec.ts create mode 100644 packages/medusa/src/utils/csv-cell-content-formatter.ts diff --git a/.changeset/small-walls-poke.md b/.changeset/small-walls-poke.md new file mode 100644 index 0000000000..5cad7c820d --- /dev/null +++ b/.changeset/small-walls-poke.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +Handle new line char in csv cell and fix import strategy diff --git a/integration-tests/api/__tests__/batch-jobs/product/export.js b/integration-tests/api/__tests__/batch-jobs/product/export.js index e37918e6f1..5e2d3c37bd 100644 --- a/integration-tests/api/__tests__/batch-jobs/product/export.js +++ b/integration-tests/api/__tests__/batch-jobs/product/export.js @@ -173,6 +173,110 @@ describe("Batch job of product-export type", () => { expect(lineColumn[25]).toBe(productPayload.variants[0].sku) }) + it("should export a csv file containing the expected products including new line char in the cells", async () => { + jest.setTimeout(1000000) + const api = useApi() + + const productPayload = { + title: "Test export product", + description: "test-product-description\ntest line 2", + type: { value: "test-type" }, + images: ["test-image.png", "test-image-2.png"], + collection_id: "test-collection", + tags: [{ value: "123" }, { value: "456" }], + options: [{ title: "size" }, { title: "color" }], + variants: [ + { + title: "Test variant", + inventory_quantity: 10, + sku: "test-variant-sku-product-export", + prices: [ + { + currency_code: "usd", + amount: 100, + }, + { + currency_code: "eur", + amount: 45, + }, + { + currency_code: "dkk", + amount: 30, + }, + ], + options: [{ value: "large" }, { value: "green" }], + }, + ], + } + const createProductRes = await api.post( + "/admin/products", + productPayload, + adminReqConfig + ) + const productId = createProductRes.data.product.id + const variantId = createProductRes.data.product.variants[0].id + + const batchPayload = { + type: "product-export", + context: { + filterable_fields: { + title: "Test export product", + }, + }, + } + const batchJobRes = await api.post( + "/admin/batch-jobs", + batchPayload, + adminReqConfig + ) + const batchJobId = batchJobRes.data.batch_job.id + + expect(batchJobId).toBeTruthy() + + // Pull to check the status until it is completed + let batchJob + let shouldContinuePulling = true + while (shouldContinuePulling) { + const res = await api.get( + `/admin/batch-jobs/${batchJobId}`, + adminReqConfig + ) + + await new Promise((resolve, _) => { + setTimeout(resolve, 1000) + }) + + batchJob = res.data.batch_job + shouldContinuePulling = !( + batchJob.status === "completed" || batchJob.status === "failed" + ) + } + + expect(batchJob.status).toBe("completed") + + exportFilePath = path.resolve(__dirname, batchJob.result.file_key) + const isFileExists = (await fs.stat(exportFilePath)).isFile() + + expect(isFileExists).toBeTruthy() + + const fileSize = (await fs.stat(exportFilePath)).size + expect(batchJob.result?.file_size).toBe(fileSize) + + const data = (await fs.readFile(exportFilePath)).toString() + const [, ...lines] = data.split("\r\n").filter((l) => l) + + expect(lines.length).toBe(1) + + const lineColumn = lines[0].split(";") + + expect(lineColumn[0]).toBe(productId) + expect(lineColumn[2]).toBe(productPayload.title) + expect(lineColumn[4]).toBe(`"${productPayload.description}"`) + expect(lineColumn[23]).toBe(variantId) + expect(lineColumn[24]).toBe(productPayload.variants[0].title) + expect(lineColumn[25]).toBe(productPayload.variants[0].sku) + }) + it("should export a csv file containing a limited number of products", async () => { jest.setTimeout(1000000) const api = useApi() diff --git a/integration-tests/api/__tests__/batch-jobs/product/import.js b/integration-tests/api/__tests__/batch-jobs/product/import.js index 65e2ea9c4b..c424237ae1 100644 --- a/integration-tests/api/__tests__/batch-jobs/product/import.js +++ b/integration-tests/api/__tests__/batch-jobs/product/import.js @@ -111,145 +111,148 @@ describe("Product import batch job", () => { const productsResponse = await api.get("/admin/products", adminReqConfig) expect(productsResponse.data.count).toBe(2) - expect(productsResponse.data.products).toEqual([ - expect.objectContaining({ - id: "O6S1YQ6mKm", - title: "Test product", - description: "test-product-description-1", - handle: "test-product-product-1", - is_giftcard: false, - status: "draft", - thumbnail: "test-image.png", - variants: [ - expect.objectContaining({ - title: "Test variant", - product_id: "O6S1YQ6mKm", - sku: "test-sku-1", - barcode: "test-barcode-1", - ean: null, - upc: null, - inventory_quantity: 10, - prices: [ - expect.objectContaining({ - currency_code: "eur", - amount: 100, - region_id: "region-product-import-0", - }), - expect.objectContaining({ - currency_code: "usd", - amount: 110, - }), - expect.objectContaining({ - currency_code: "dkk", - amount: 130, - region_id: "region-product-import-1", - }), - ], - options: [ - expect.objectContaining({ - value: "option 1 value red", - }), - expect.objectContaining({ - value: "option 2 value 1", - }), - ], - }), - ], - images: [ - expect.objectContaining({ - url: "test-image.png", - }), - ], - options: [ - expect.objectContaining({ - title: "test-option-1", - product_id: "O6S1YQ6mKm", - }), - expect.objectContaining({ - title: "test-option-2", - product_id: "O6S1YQ6mKm", - }), - ], - tags: [ - expect.objectContaining({ - value: "123_1", - }), - ], - }), - expect.objectContaining({ - id: "5VxiEkmnPV", - title: "Test product", - description: "test-product-description", - handle: "test-product-product-2", - is_giftcard: false, - status: "draft", - thumbnail: "test-image.png", - profile_id: expect.any(String), - variants: [ - expect.objectContaining({ - title: "Test variant", - product_id: "5VxiEkmnPV", - sku: "test-sku-2", - barcode: "test-barcode-2", - ean: null, - upc: null, - inventory_quantity: 10, - allow_backorder: false, - manage_inventory: true, - prices: [ - expect.objectContaining({ - currency_code: "dkk", - amount: 110, - region_id: "region-product-import-2", - }), - ], - options: [ - expect.objectContaining({ - value: "Option 1 value 1", - }), - ], - }), - expect.objectContaining({ - title: "Test variant", - product_id: "5VxiEkmnPV", - sku: "test-sku-3", - barcode: "test-barcode-3", - ean: null, - upc: null, - inventory_quantity: 10, - allow_backorder: false, - manage_inventory: true, - prices: [ - expect.objectContaining({ - currency_code: "usd", - amount: 120, - region_id: null, - }), - ], - options: [ - expect.objectContaining({ - value: "Option 1 Value blue", - }), - ], - }), - ], - images: [ - expect.objectContaining({ - url: "test-image.png", - }), - ], - options: [ - expect.objectContaining({ - title: "test-option", - product_id: "5VxiEkmnPV", - }), - ], - tags: [ - expect.objectContaining({ - value: "123", - }), - ], - }), - ]) + expect(productsResponse.data.products).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "O6S1YQ6mKm", + title: "Test product", + description: + "Hopper Stripes Bedding, available as duvet cover, pillow sham and sheet.\\n100% organic cotton, soft and crisp to the touch. Made in Portugal.", + handle: "test-product-product-1", + is_giftcard: false, + status: "draft", + thumbnail: "test-image.png", + variants: [ + expect.objectContaining({ + title: "Test variant", + product_id: "O6S1YQ6mKm", + sku: "test-sku-1", + barcode: "test-barcode-1", + ean: null, + upc: null, + inventory_quantity: 10, + prices: [ + expect.objectContaining({ + currency_code: "eur", + amount: 100, + region_id: "region-product-import-0", + }), + expect.objectContaining({ + currency_code: "usd", + amount: 110, + }), + expect.objectContaining({ + currency_code: "dkk", + amount: 130, + region_id: "region-product-import-1", + }), + ], + options: [ + expect.objectContaining({ + value: "option 1 value red", + }), + expect.objectContaining({ + value: "option 2 value 1", + }), + ], + }), + ], + images: [ + expect.objectContaining({ + url: "test-image.png", + }), + ], + options: [ + expect.objectContaining({ + title: "test-option-1", + product_id: "O6S1YQ6mKm", + }), + expect.objectContaining({ + title: "test-option-2", + product_id: "O6S1YQ6mKm", + }), + ], + tags: [ + expect.objectContaining({ + value: "123_1", + }), + ], + }), + expect.objectContaining({ + id: "5VxiEkmnPV", + title: "Test product", + description: "test-product-description", + handle: "test-product-product-2", + is_giftcard: false, + status: "draft", + thumbnail: "test-image.png", + profile_id: expect.any(String), + variants: [ + expect.objectContaining({ + title: "Test variant", + product_id: "5VxiEkmnPV", + sku: "test-sku-2", + barcode: "test-barcode-2", + ean: null, + upc: null, + inventory_quantity: 10, + allow_backorder: false, + manage_inventory: true, + prices: [ + expect.objectContaining({ + currency_code: "dkk", + amount: 110, + region_id: "region-product-import-2", + }), + ], + options: [ + expect.objectContaining({ + value: "Option 1 value 1", + }), + ], + }), + expect.objectContaining({ + title: "Test variant", + product_id: "5VxiEkmnPV", + sku: "test-sku-3", + barcode: "test-barcode-3", + ean: null, + upc: null, + inventory_quantity: 10, + allow_backorder: false, + manage_inventory: true, + prices: [ + expect.objectContaining({ + currency_code: "usd", + amount: 120, + region_id: null, + }), + ], + options: [ + expect.objectContaining({ + value: "Option 1 Value blue", + }), + ], + }), + ], + images: [ + expect.objectContaining({ + url: "test-image.png", + }), + ], + options: [ + expect.objectContaining({ + title: "test-option", + product_id: "5VxiEkmnPV", + }), + ], + tags: [ + expect.objectContaining({ + value: "123", + }), + ], + }), + ]) + ) }) }) diff --git a/integration-tests/api/__tests__/batch-jobs/product/product-import-ss.csv b/integration-tests/api/__tests__/batch-jobs/product/product-import-ss.csv index 53044c5bbb..0316762d9c 100644 --- a/integration-tests/api/__tests__/batch-jobs/product/product-import-ss.csv +++ b/integration-tests/api/__tests__/batch-jobs/product/product-import-ss.csv @@ -1,4 +1,4 @@ -Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product Mid Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant Mid Code,Variant Material,Price ImportLand [EUR],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url,Sales Channel 1 Name,Sales Channel 2 Name,Sales Channel 1 Id,Sales Channel 2 Id +Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product MID Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant MID Code,Variant Material,Price ImportLand [EUR],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url,Sales Channel 1 Name,Sales Channel 2 Name,Sales Channel 1 Id,Sales Channel 2 Id O6S1YQ6mKm,test-product-product-1,Test product,,test-product-description-1,draft,,,,,,,,,,Test collection 1,test-collection1,test-type-1,123_1,TRUE,,profile_1,profile_type_1,,Test variant,test-sku-1,test-barcode-1,10,FALSE,TRUE,,,,,,,,,100,110,130,,test-option-1,option 1 value red,test-option-2,option 2 value 1,test-image.png,Import Sales Channel 1,Import Sales Channel 2,, 5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,,Test variant,test-sku-2,test-barcode-2,10,FALSE,TRUE,,,,,,,,,,,,110,test-option,Option 1 value 1,,,test-image.png,,,, 5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,,Test variant,test-sku-3,test-barcode-3,10,FALSE,TRUE,,,,,,,,,,120,,,test-option,Option 1 Value blue,,,test-image.png,,,, \ No newline at end of file diff --git a/integration-tests/api/__tests__/batch-jobs/product/product-import.csv b/integration-tests/api/__tests__/batch-jobs/product/product-import.csv index ee70778ceb..d7579724ee 100644 --- a/integration-tests/api/__tests__/batch-jobs/product/product-import.csv +++ b/integration-tests/api/__tests__/batch-jobs/product/product-import.csv @@ -1,4 +1,4 @@ -Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product Mid Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant Mid Code,Variant Material,Price ImportLand [EUR],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url -O6S1YQ6mKm,test-product-product-1,Test product,,test-product-description-1,draft,,,,,,,,,,Test collection 1,test-collection1,test-type-1,123_1,TRUE,,profile_1,profile_type_1,,Test variant,test-sku-1,test-barcode-1,10,FALSE,TRUE,,,,,,,,,100,110,130,,test-option-1,option 1 value red,test-option-2,option 2 value 1,test-image.png +Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product MID Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant MID Code,Variant Material,Price ImportLand [EUR],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url +O6S1YQ6mKm,test-product-product-1,Test product,,"Hopper Stripes Bedding, available as duvet cover, pillow sham and sheet.\n100% organic cotton, soft and crisp to the touch. Made in Portugal.",draft,,,,,,,,,,Test collection 1,test-collection1,test-type-1,123_1,TRUE,,profile_1,profile_type_1,,Test variant,test-sku-1,test-barcode-1,10,FALSE,TRUE,,,,,,,,,100,110,130,,test-option-1,option 1 value red,test-option-2,option 2 value 1,test-image.png 5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,,Test variant,test-sku-2,test-barcode-2,10,FALSE,TRUE,,,,,,,,,,,,110,test-option,Option 1 value 1,,,test-image.png 5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,,Test variant,test-sku-3,test-barcode-3,10,FALSE,TRUE,,,,,,,,,,120,,,test-option,Option 1 Value blue,,,test-image.png \ No newline at end of file diff --git a/integration-tests/api/src/services/local-file-service.js b/integration-tests/api/src/services/local-file-service.js index 500e4c6e50..1bc25a2013 100644 --- a/integration-tests/api/src/services/local-file-service.js +++ b/integration-tests/api/src/services/local-file-service.js @@ -1,12 +1,12 @@ import { AbstractFileService } from "@medusajs/medusa" import stream from "stream" +import { resolve } from "path" import * as fs from "fs" -import * as path from "path" +import mkdirp from "mkdirp" export default class LocalFileService extends AbstractFileService { - // eslint-disable-next-line no-empty-pattern constructor({}, options) { - super({}) + super({}, options) this.upload_dir_ = process.env.UPLOAD_DIR ?? options.upload_dir ?? "uploads/images" @@ -15,49 +15,56 @@ export default class LocalFileService extends AbstractFileService { } } - async upload(file) { - const uploadPath = path.join( - this.upload_dir_, - path.dirname(file.originalname) - ) + upload(file) { + return new Promise((resolvePromise, reject) => { + const path = resolve(this.upload_dir_, file.originalname) - if (!fs.existsSync(uploadPath)) { - fs.mkdirSync(uploadPath, { recursive: true }) - } - - const filePath = path.resolve(this.upload_dir_, file.originalname) - fs.writeFile(filePath, "", (error) => { - if (error) { - throw error + let content = "" + if (file.filename) { + content = fs.readFileSync( + resolve(process.cwd(), "uploads", file.filename) + ) } - }) - return { url: filePath } - } - async delete({ name }) { - return new Promise((resolve, _) => { - const path = resolve(this.upload_dir_, name) - fs.unlink(path, (err) => { + const pathSegments = path.split("/") + pathSegments.splice(-1) + const dirname = pathSegments.join("/") + mkdirp.sync(dirname, { recursive: true }) + + fs.writeFile(path, content.toString(), (err) => { if (err) { - throw err + reject(err) } - resolve("file unlinked") + resolvePromise({ url: path }) + }) + }) + } + + delete({ fileKey }) { + return new Promise((resolvePromise, reject) => { + const path = resolve(this.upload_dir_, fileKey) + fs.unlink(path, (err) => { + if (err) { + reject(err) + } + + resolvePromise("file unlinked") }) }) } async getUploadStreamDescriptor({ name, ext }) { const fileKey = `${name}.${ext}` - const filePath = path.resolve(this.upload_dir_, fileKey) + const path = resolve(this.upload_dir_, fileKey) - const isFileExists = fs.existsSync(filePath) + const isFileExists = fs.existsSync(path) if (!isFileExists) { await this.upload({ originalname: fileKey }) } const pass = new stream.PassThrough() - pass.pipe(fs.createWriteStream(filePath)) + pass.pipe(fs.createWriteStream(path)) return { writeStream: pass, @@ -67,11 +74,23 @@ export default class LocalFileService extends AbstractFileService { } } - async getDownloadStream(fileData) { - const filePath = path.resolve( - this.upload_dir_, - fileData.fileKey + (fileData.ext ? `.${fileData.ext}` : "") - ) - return fs.createReadStream(filePath) + async getDownloadStream({ fileKey }) { + return new Promise((resolvePromise, reject) => { + try { + const path = resolve(this.upload_dir_, fileKey) + const data = fs.readFileSync(path) + const readable = stream.Readable() + readable._read = function () {} + readable.push(data.toString()) + readable.push(null) + resolvePromise(readable) + } catch (e) { + reject(e) + } + }) + } + + async getPresignedDownloadUrl({ fileKey }) { + return `${this.upload_dir_}/${fileKey}` } } diff --git a/packages/medusa/src/api/routes/admin/batch/create-batch-job.ts b/packages/medusa/src/api/routes/admin/batch/create-batch-job.ts index 498df2c1c0..3d786668df 100644 --- a/packages/medusa/src/api/routes/admin/batch/create-batch-job.ts +++ b/packages/medusa/src/api/routes/admin/batch/create-batch-job.ts @@ -101,15 +101,14 @@ export default async (req, res) => { const validated = await validator(AdminPostBatchesReq, req.body) const batchJobService: BatchJobService = req.scope.resolve("batchJobService") - const toCreate = await batchJobService.prepareBatchJobForProcessing( - validated, - req - ) const userId = req.user.id ?? req.user.userId const manager: EntityManager = req.scope.resolve("manager") const batch_job = await manager.transaction(async (transactionManager) => { + const toCreate = await batchJobService + .withTransaction(transactionManager) + .prepareBatchJobForProcessing(validated, req) return await batchJobService.withTransaction(transactionManager).create({ ...toCreate, created_by: userId, diff --git a/packages/medusa/src/services/batch-job.ts b/packages/medusa/src/services/batch-job.ts index 76e164382d..9422cf5569 100644 --- a/packages/medusa/src/services/batch-job.ts +++ b/packages/medusa/src/services/batch-job.ts @@ -372,11 +372,13 @@ class BatchJobService extends TransactionBaseService { data: CreateBatchJobInput, req: Request ): Promise { - return await this.atomicPhase_(async () => { + return await this.atomicPhase_(async (transactionManager) => { const batchStrategy = this.strategyResolver_.resolveBatchJobByType( data.type ) - return await batchStrategy.prepareBatchJobForProcessing(data, req) + return await batchStrategy + .withTransaction(transactionManager) + .prepareBatchJobForProcessing(data, req) }) } } diff --git a/packages/medusa/src/services/product-variant.ts b/packages/medusa/src/services/product-variant.ts index c6121acf7a..cd6ca44308 100644 --- a/packages/medusa/src/services/product-variant.ts +++ b/packages/medusa/src/services/product-variant.ts @@ -303,7 +303,7 @@ class ProductVariantService extends BaseService { const variantRes = await variantRepo.findOne({ where: { id: variantOrVariantId as string }, }) - if (typeof variant === "undefined") { + if (!isDefined(variantRes)) { throw new MedusaError( MedusaError.Types.NOT_FOUND, `Variant with id ${variantOrVariantId} was not found` diff --git a/packages/medusa/src/strategies/__fixtures__/product-export-data.ts b/packages/medusa/src/strategies/__fixtures__/product-export-data.ts index 9e2ef61887..3fe70ea8e4 100644 --- a/packages/medusa/src/strategies/__fixtures__/product-export-data.ts +++ b/packages/medusa/src/strategies/__fixtures__/product-export-data.ts @@ -12,7 +12,11 @@ const variantIds = [ export const productsToExport = [ { sales_channels: [ - { id: IdMap.getId("sc_1"), name: "SC 1", description: "SC 1" }, + { + id: IdMap.getId("sc_1"), + name: "SC 1", + description: "SC 1\nSC 1 second line\nSC 1 third line\nSC 1 forth line", + }, ], collection: { created_at: "randomString", @@ -26,7 +30,8 @@ export const productsToExport = [ collection_id: IdMap.getId("product-export-collection_1"), created_at: "randomString", deleted_at: null, - description: "test-product-description-1", + description: + "test-product-description-1\ntest-product-description-1 second line\ntest-product-description-1 third line\nforth line", discountable: true, external_id: null, handle: "test-product-product-1", diff --git a/packages/medusa/src/strategies/__tests__/batch-jobs/product/__snapshots__/export.ts.snap b/packages/medusa/src/strategies/__tests__/batch-jobs/product/__snapshots__/export.ts.snap index e2ba7d7ad4..1d6c2a442b 100644 --- a/packages/medusa/src/strategies/__tests__/batch-jobs/product/__snapshots__/export.ts.snap +++ b/packages/medusa/src/strategies/__tests__/batch-jobs/product/__snapshots__/export.ts.snap @@ -2,9 +2,9 @@ exports[`Product export strategy should process the batch job and generate the appropriate output 1`] = ` Array [ - "Product ID;Product Handle;Product Title;Product Subtitle;Product Description;Product Status;Product Thumbnail;Product Weight;Product Length;Product Width;Product Height;Product HS Code;Product Origin Country;Product MID Code;Product Material;Product Collection Title;Product Collection Handle;Product Type;Product Tags;Product Discountable;Product External ID;Product Profile Name;Product Profile Type;Variant ID;Variant Title;Variant SKU;Variant Barcode;Variant Inventory Quantity;Variant Allow backorder;Variant Manage inventory;Variant Weight;Variant Length;Variant Width;Variant Height;Variant HS Code;Variant Origin Country;Variant MID Code;Variant Material;Price france [USD];Price USD;Price denmark [DKK];Price Denmark [DKK];Option 1 Name;Option 1 Value;Option 2 Name;Option 2 Value;Image 1 Url + "Product id;Product Handle;Product Title;Product Subtitle;Product Description;Product Status;Product Thumbnail;Product Weight;Product Length;Product Width;Product Height;Product HS Code;Product Origin Country;Product MID Code;Product Material;Product Collection Title;Product Collection Handle;Product Type;Product Tags;Product Discountable;Product External ID;Product Profile Name;Product Profile Type;Variant id;Variant Title;Variant SKU;Variant Barcode;Variant Inventory Quantity;Variant Allow backorder;Variant Manage inventory;Variant Weight;Variant Length;Variant Width;Variant Height;Variant HS Code;Variant Origin Country;Variant MID Code;Variant Material;Price france [USD];Price USD;Price denmark [DKK];Price Denmark [DKK];Option 1 Name;Option 1 Value;Option 2 Name;Option 2 Value;Image 1 Url ", - "product-export-strategy-product-1;test-product-product-1;Test product;;test-product-description-1;draft;;;;;;;;;;Test collection 1;test-collection1;test-type-1;123_1;true;;profile_1;profile_type_1;product-export-strategy-variant-1;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;100;110;130;;test-option-1;option 1 value 1;test-option-2;option 2 value 1;test-image.png + "product-export-strategy-product-1;test-product-product-1;Test product;;\\"test-product-description-1\ntest-product-description-1 second line\ntest-product-description-1 third line\nforth line\\";draft;;;;;;;;;;Test collection 1;test-collection1;test-type-1;123_1;true;;profile_1;profile_type_1;product-export-strategy-variant-1;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;100;110;130;;test-option-1;option 1 value 1;test-option-2;option 2 value 1;test-image.png ", "product-export-strategy-product-2;test-product-product-2;Test product;;test-product-description;draft;;;;;;;;;;Test collection;test-collection2;test-type;123;true;;profile_2;profile_type_2;product-export-strategy-variant-2;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;;;;110;test-option;Option 1 value 1;;;test-image.png ", @@ -15,9 +15,9 @@ Array [ exports[`Product export strategy with sales channels should process the batch job and generate the appropriate output 1`] = ` Array [ - "Product ID;Product Handle;Product Title;Product Subtitle;Product Description;Product Status;Product Thumbnail;Product Weight;Product Length;Product Width;Product Height;Product HS Code;Product Origin Country;Product MID Code;Product Material;Product Collection Title;Product Collection Handle;Product Type;Product Tags;Product Discountable;Product External ID;Product Profile Name;Product Profile Type;Variant ID;Variant Title;Variant SKU;Variant Barcode;Variant Inventory Quantity;Variant Allow backorder;Variant Manage inventory;Variant Weight;Variant Length;Variant Width;Variant Height;Variant HS Code;Variant Origin Country;Variant MID Code;Variant Material;Price france [USD];Price USD;Price denmark [DKK];Price Denmark [DKK];Option 1 Name;Option 1 Value;Option 2 Name;Option 2 Value;Image 1 Url;Sales channel 1 Name;Sales channel 1 Description;Sales channel 2 Name;Sales channel 2 Description + "Product id;Product Handle;Product Title;Product Subtitle;Product Description;Product Status;Product Thumbnail;Product Weight;Product Length;Product Width;Product Height;Product HS Code;Product Origin Country;Product MID Code;Product Material;Product Collection Title;Product Collection Handle;Product Type;Product Tags;Product Discountable;Product External ID;Product Profile Name;Product Profile Type;Variant id;Variant Title;Variant SKU;Variant Barcode;Variant Inventory Quantity;Variant Allow backorder;Variant Manage inventory;Variant Weight;Variant Length;Variant Width;Variant Height;Variant HS Code;Variant Origin Country;Variant MID Code;Variant Material;Price france [USD];Price USD;Price denmark [DKK];Price Denmark [DKK];Option 1 Name;Option 1 Value;Option 2 Name;Option 2 Value;Image 1 Url;Sales channel 1 Name;Sales channel 1 Description;Sales channel 2 Name;Sales channel 2 Description ", - "product-export-strategy-product-1;test-product-product-1;Test product;;test-product-description-1;draft;;;;;;;;;;Test collection 1;test-collection1;test-type-1;123_1;true;;profile_1;profile_type_1;product-export-strategy-variant-1;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;100;110;130;;test-option-1;option 1 value 1;test-option-2;option 2 value 1;test-image.png;SC 1;SC 1;; + "product-export-strategy-product-1;test-product-product-1;Test product;;\\"test-product-description-1\ntest-product-description-1 second line\ntest-product-description-1 third line\nforth line\\";draft;;;;;;;;;;Test collection 1;test-collection1;test-type-1;123_1;true;;profile_1;profile_type_1;product-export-strategy-variant-1;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;100;110;130;;test-option-1;option 1 value 1;test-option-2;option 2 value 1;test-image.png;SC 1;\\"SC 1\nSC 1 second line\nSC 1 third line\nSC 1 forth line\\";; ", "product-export-strategy-product-2;test-product-product-2;Test product;;test-product-description;draft;;;;;;;;;;Test collection;test-collection2;test-type;123;true;;profile_2;profile_type_2;product-export-strategy-variant-2;Test variant;test-sku;test-barcode;10;false;true;;;;;;;;;;;;110;test-option;Option 1 value 1;;;test-image.png;SC 1;SC 1;SC 2;SC 2 ", diff --git a/packages/medusa/src/strategies/__tests__/batch-jobs/product/export.ts b/packages/medusa/src/strategies/__tests__/batch-jobs/product/export.ts index 402a4ffc5b..49e76b7a94 100644 --- a/packages/medusa/src/strategies/__tests__/batch-jobs/product/export.ts +++ b/packages/medusa/src/strategies/__tests__/batch-jobs/product/export.ts @@ -3,18 +3,24 @@ import { IdMap, MockManager } from "medusa-test-utils" import { User } from "../../../../models" import { BatchJobStatus } from "../../../../types/batch-job" import { productsToExport } from "../../../__fixtures__/product-export-data" -import { AdminPostBatchesReq, defaultAdminProductRelations } from "../../../../api" +import { + AdminPostBatchesReq, + defaultAdminProductRelations, +} from "../../../../api" import { ProductExportBatchJob } from "../../../batch-jobs/product" import { Request } from "express" -import { FlagRouter } from "../../../../utils/flag-router"; -import SalesChannelFeatureFlag from "../../../../loaders/feature-flags/sales-channels"; +import { FlagRouter } from "../../../../utils/flag-router" +import SalesChannelFeatureFlag from "../../../../loaders/feature-flags/sales-channels" const productServiceMock = { withTransaction: function () { return this }, list: jest.fn().mockImplementation(() => Promise.resolve(productsToExport)), - count: jest.fn().mockImplementation(() => Promise.resolve(productsToExport.length)), + + count: jest + .fn() + .mockImplementation(() => Promise.resolve(productsToExport.length)), listAndCount: jest.fn().mockImplementation(() => { return Promise.resolve([productsToExport, productsToExport.length]) }), @@ -39,31 +45,31 @@ describe("Product export strategy", () => { write: (data: string) => { outputDataStorage.push(data) }, - end: () => void 0 + end: () => void 0, }, promise: Promise.resolve(), - fileKey: 'product-export.csv' + fileKey: "product-export.csv", }) }), withTransaction: function () { return this - } + }, } let fakeJob = { id: IdMap.getId("product-export-job"), - type: 'product-export', + type: "product-export", created_by: IdMap.getId("product-export-job-creator"), created_by_user: {} as User, context: {}, result: {}, dry_run: false, - status: BatchJobStatus.PROCESSING as BatchJobStatus + status: BatchJobStatus.PROCESSING as BatchJobStatus, } as ProductExportBatchJob let canceledFakeJob = { ...fakeJob, id: "bj_failed", - status: BatchJobStatus.CANCELED + status: BatchJobStatus.CANCELED, } as ProductExportBatchJob const batchJobServiceMock = { @@ -76,7 +82,7 @@ describe("Product export strategy", () => { ...canceledFakeJob, ...data, context: { ...canceledFakeJob?.context, ...data?.context }, - result: { ...canceledFakeJob?.result, ...data?.result } + result: { ...canceledFakeJob?.result, ...data?.result }, } return Promise.resolve(canceledFakeJob) @@ -86,7 +92,7 @@ describe("Product export strategy", () => { ...fakeJob, ...data, context: { ...fakeJob?.context, ...data?.context }, - result: { ...fakeJob?.result, ...data?.result } + result: { ...fakeJob?.result, ...data?.result }, } return Promise.resolve(fakeJob) @@ -100,14 +106,12 @@ describe("Product export strategy", () => { return Promise.resolve(fakeJob) }), retrieve: jest.fn().mockImplementation((id) => { - const targetFakeJob = id === "bj_failed" - ? canceledFakeJob - : fakeJob + const targetFakeJob = id === "bj_failed" ? canceledFakeJob : fakeJob return Promise.resolve(targetFakeJob) }), setFailed: jest.fn().mockImplementation((...args) => { console.error(...args) - }) + }), } const productExportStrategy = new ProductExportStrategy({ @@ -118,11 +122,14 @@ describe("Product export strategy", () => { featureFlagRouter: new FlagRouter({}), }) - it('should generate the appropriate template', async () => { - await productExportStrategy.prepareBatchJobForProcessing(fakeJob, {} as Request) + it("should generate the appropriate template", async () => { + await productExportStrategy.prepareBatchJobForProcessing( + fakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(fakeJob.id) const template = await productExportStrategy.buildHeader(fakeJob) - expect(template).toMatch(/.*Product ID.*/) + expect(template).toMatch(/.*Product id.*/) expect(template).toMatch(/.*Product Handle.*/) expect(template).toMatch(/.*Product Title.*/) expect(template).toMatch(/.*Product Subtitle.*/) @@ -147,7 +154,7 @@ describe("Product export strategy", () => { expect(template).toMatch(/.*Product Profile Type.*/) expect(template).toMatch(/.*Product Profile Type.*/) - expect(template).toMatch(/.*Variant ID.*/) + expect(template).toMatch(/.*Variant id.*/) expect(template).toMatch(/.*Variant Title.*/) expect(template).toMatch(/.*Variant SKU.*/) expect(template).toMatch(/.*Variant Barcode.*/) @@ -180,26 +187,29 @@ describe("Product export strategy", () => { expect(template).toMatch(/.*Image 1 Url.*/) }) - it('should process the batch job and generate the appropriate output', async () => { - await productExportStrategy.prepareBatchJobForProcessing(fakeJob, {} as Request) + it("should process the batch job and generate the appropriate output", async () => { + await productExportStrategy.prepareBatchJobForProcessing( + fakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(fakeJob.id) await productExportStrategy.processJob(fakeJob.id) expect(outputDataStorage).toMatchSnapshot() expect((fakeJob.result as any).file_key).toBeDefined() }) - it('should prepare the job to be pre proccessed', async () => { + it("should prepare the job to be pre processed", async () => { const fakeJob1: AdminPostBatchesReq = { - type: 'product-export', + type: "product-export", context: { limit: 10, offset: 10, expand: "variants", fields: "title", order: "-title", - filterable_fields: { title: "test" } + filterable_fields: { title: "test" }, }, - dry_run: false + dry_run: false, } const output1 = await productExportStrategy.prepareBatchJobForProcessing( @@ -207,21 +217,23 @@ describe("Product export strategy", () => { {} as Express.Request ) - expect(output1.context).toEqual(expect.objectContaining({ - list_config: { - select: ["title", "created_at", "id"], - order: { title: "DESC" }, - relations: ["variants"], - skip: 10, - take: 10, - }, - filterable_fields: { title: "test" } - })) + expect(output1.context).toEqual( + expect.objectContaining({ + list_config: { + select: ["title", "created_at", "id"], + order: { title: "DESC" }, + relations: ["variants"], + skip: 10, + take: 10, + }, + filterable_fields: { title: "test" }, + }) + ) const fakeJob2: AdminPostBatchesReq = { - type: 'product-export', + type: "product-export", context: {}, - dry_run: false + dry_run: false, } const output2 = await productExportStrategy.prepareBatchJobForProcessing( @@ -229,19 +241,21 @@ describe("Product export strategy", () => { {} as Express.Request ) - expect(output2.context).toEqual(expect.objectContaining({ - list_config: { - select: undefined, - order: { created_at: "DESC" }, - relations: [ - ...defaultAdminProductRelations, - "variants.prices.region" - ], - skip: 0, - take: 50, - }, - filterable_fields: undefined - })) + expect(output2.context).toEqual( + expect.objectContaining({ + list_config: { + select: undefined, + order: { created_at: "DESC" }, + relations: [ + ...defaultAdminProductRelations, + "variants.prices.region", + ], + skip: 0, + take: 50, + }, + filterable_fields: undefined, + }) + ) }) it("should always provide a file_key even with no data", async () => { @@ -253,7 +267,10 @@ describe("Product export strategy", () => { featureFlagRouter: new FlagRouter({}), }) - await productExportStrategy.prepareBatchJobForProcessing(fakeJob, {} as Request) + await productExportStrategy.prepareBatchJobForProcessing( + fakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(fakeJob.id) await productExportStrategy.processJob(fakeJob.id) @@ -269,7 +286,10 @@ describe("Product export strategy", () => { featureFlagRouter: new FlagRouter({}), }) - await productExportStrategy.prepareBatchJobForProcessing(canceledFakeJob, {} as Request) + await productExportStrategy.prepareBatchJobForProcessing( + canceledFakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(canceledFakeJob.id) await productExportStrategy.processJob(canceledFakeJob.id) @@ -288,31 +308,31 @@ describe("Product export strategy with sales channels", () => { write: (data: string) => { outputDataStorage.push(data) }, - end: () => void 0 + end: () => void 0, }, promise: Promise.resolve(), - fileKey: 'product-export.csv' + fileKey: "product-export.csv", }) }), withTransaction: function () { return this - } + }, } let fakeJob = { id: IdMap.getId("product-export-job"), - type: 'product-export', + type: "product-export", created_by: IdMap.getId("product-export-job-creator"), created_by_user: {} as User, context: {}, result: {}, dry_run: false, - status: BatchJobStatus.PROCESSING as BatchJobStatus + status: BatchJobStatus.PROCESSING as BatchJobStatus, } as ProductExportBatchJob let canceledFakeJob = { ...fakeJob, id: "bj_failed", - status: BatchJobStatus.CANCELED + status: BatchJobStatus.CANCELED, } as ProductExportBatchJob const batchJobServiceMock = { @@ -325,7 +345,7 @@ describe("Product export strategy with sales channels", () => { ...canceledFakeJob, ...data, context: { ...canceledFakeJob?.context, ...data?.context }, - result: { ...canceledFakeJob?.result, ...data?.result } + result: { ...canceledFakeJob?.result, ...data?.result }, } return Promise.resolve(canceledFakeJob) @@ -335,7 +355,7 @@ describe("Product export strategy with sales channels", () => { ...fakeJob, ...data, context: { ...fakeJob?.context, ...data?.context }, - result: { ...fakeJob?.result, ...data?.result } + result: { ...fakeJob?.result, ...data?.result }, } return Promise.resolve(fakeJob) @@ -349,14 +369,12 @@ describe("Product export strategy with sales channels", () => { return Promise.resolve(fakeJob) }), retrieve: jest.fn().mockImplementation((id) => { - const targetFakeJob = id === "bj_failed" - ? canceledFakeJob - : fakeJob + const targetFakeJob = id === "bj_failed" ? canceledFakeJob : fakeJob return Promise.resolve(targetFakeJob) }), setFailed: jest.fn().mockImplementation((...args) => { console.error(...args) - }) + }), } const productExportStrategy = new ProductExportStrategy({ @@ -369,11 +387,14 @@ describe("Product export strategy with sales channels", () => { }), }) - it('should generate the appropriate template', async () => { - await productExportStrategy.prepareBatchJobForProcessing(fakeJob, {} as Request) + it("should generate the appropriate template", async () => { + await productExportStrategy.prepareBatchJobForProcessing( + fakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(fakeJob.id) const template = await productExportStrategy.buildHeader(fakeJob) - expect(template).toMatch(/.*Product ID.*/) + expect(template).toMatch(/.*Product id.*/) expect(template).toMatch(/.*Product Handle.*/) expect(template).toMatch(/.*Product Title.*/) expect(template).toMatch(/.*Product Subtitle.*/) @@ -398,7 +419,7 @@ describe("Product export strategy with sales channels", () => { expect(template).toMatch(/.*Product Profile Type.*/) expect(template).toMatch(/.*Product Profile Type.*/) - expect(template).toMatch(/.*Variant ID.*/) + expect(template).toMatch(/.*Variant id.*/) expect(template).toMatch(/.*Variant Title.*/) expect(template).toMatch(/.*Variant SKU.*/) expect(template).toMatch(/.*Variant Barcode.*/) @@ -431,11 +452,14 @@ describe("Product export strategy with sales channels", () => { expect(template).toMatch(/.*Image 1 Url.*/) }) - it('should process the batch job and generate the appropriate output', async () => { - await productExportStrategy.prepareBatchJobForProcessing(fakeJob, {} as Request) + it("should process the batch job and generate the appropriate output", async () => { + await productExportStrategy.prepareBatchJobForProcessing( + fakeJob, + {} as Request + ) await productExportStrategy.preProcessBatchJob(fakeJob.id) await productExportStrategy.processJob(fakeJob.id) expect(outputDataStorage).toMatchSnapshot() expect((fakeJob.result as any).file_key).toBeDefined() }) -}) \ No newline at end of file +}) diff --git a/packages/medusa/src/strategies/__tests__/batch-jobs/product/import.ts b/packages/medusa/src/strategies/__tests__/batch-jobs/product/import.ts index 72bd0095a4..f13463ad50 100644 --- a/packages/medusa/src/strategies/__tests__/batch-jobs/product/import.ts +++ b/packages/medusa/src/strategies/__tests__/batch-jobs/product/import.ts @@ -32,7 +32,7 @@ let fakeJob = { } async function* generateCSVDataForStream() { - yield "Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product Mid Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant Mid Code,Variant Material,Price france [USD],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url\n" + yield "Product id,Product Handle,Product Title,Product Subtitle,Product Description,Product Status,Product Thumbnail,Product Weight,Product Length,Product Width,Product Height,Product HS Code,Product Origin Country,Product MID Code,Product Material,Product Collection Title,Product Collection Handle,Product Type,Product Tags,Product Discountable,Product External ID,Product Profile Name,Product Profile Type,Variant id,Variant Title,Variant SKU,Variant Barcode,Variant Inventory Quantity,Variant Allow backorder,Variant Manage inventory,Variant Weight,Variant Length,Variant Width,Variant Height,Variant HS Code,Variant Origin Country,Variant MID Code,Variant Material,Price france [USD],Price USD,Price denmark [DKK],Price Denmark [DKK],Option 1 Name,Option 1 Value,Option 2 Name,Option 2 Value,Image 1 Url\n" yield "O6S1YQ6mKm,test-product-product-1,Test product,,test-product-description-1,draft,,,,,,,,,,Test collection 1,test-collection1,test-type-1,123_1,TRUE,,profile_1,profile_type_1,SebniWTDeC,Test variant,test-sku-1,test-barcode-1,10,FALSE,TRUE,,,,,,,,,100,110,130,,test-option-1,option 1 value red,test-option-2,option 2 value 1,test-image.png\n" yield "5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,CaBp7amx3r,Test variant,test-sku-2,test-barcode-2,10,FALSE,TRUE,,,,,,,,,,,,110,test-option,Option 1 value 1,,,test-image.png\n" yield "5VxiEkmnPV,test-product-product-2,Test product,,test-product-description,draft,,,,,,,,,,Test collection,test-collection2,test-type,123,TRUE,,profile_2,profile_type_2,3SS1MHGDEJ,Test variant,test-sku-3,test-barcode-3,10,FALSE,TRUE,,,,,,,,,,120,,,test-option,Option 1 Value blue,,,test-image.png\n" @@ -86,6 +86,9 @@ const productServiceMock = { } const shippingProfileServiceMock = { + withTransaction: function () { + return this + }, retrieveDefault: jest.fn().mockImplementation((_data) => { return Promise.resolve({ id: "default_shipping_profile" }) }), diff --git a/packages/medusa/src/strategies/batch-jobs/product/export.ts b/packages/medusa/src/strategies/batch-jobs/product/export.ts index ef88c3ac13..fba433d06e 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/export.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/export.ts @@ -15,6 +15,7 @@ import { import { FindProductConfig } from "../../../types/product" import { FlagRouter } from "../../../utils/flag-router" import SalesChannelFeatureFlag from "../../../loaders/feature-flags/sales-channels" +import { csvCellContentFormatter } from "../../../utils" type InjectedDependencies = { manager: EntityManager @@ -426,10 +427,16 @@ export default class ProductExportStrategy extends AbstractBatchJobStrategy { const variantLineData: string[] = [] for (const [, columnSchema] of this.columnDescriptors.entries()) { if (columnSchema.entityName === "product") { - variantLineData.push(columnSchema.accessor(product)) + const formattedContent = csvCellContentFormatter( + columnSchema.accessor(product) + ) + variantLineData.push(formattedContent) } if (columnSchema.entityName === "variant") { - variantLineData.push(columnSchema.accessor(variant)) + const formattedContent = csvCellContentFormatter( + columnSchema.accessor(variant) + ) + variantLineData.push(formattedContent) } } outputLineData.push(variantLineData.join(this.DELIMITER_) + this.NEWLINE_) @@ -461,6 +468,12 @@ export default class ProductExportStrategy extends AbstractBatchJobStrategy { * The number of item of a relation can vary between 0-Infinity and therefore the number of columns * that will be added to the export correspond to that number * @param products - The main entity to get the relation shape from + * @return ({ + * optionColumnCount: number + * imageColumnCount: number + * salesChannelsColumnCount: number + * pricesData: Set + * }) * @private */ private getProductRelationsDynamicColumnsShape(products: Product[]): { diff --git a/packages/medusa/src/strategies/batch-jobs/product/import.ts b/packages/medusa/src/strategies/batch-jobs/product/import.ts index f652b1dd63..f5d4cf5ba3 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/import.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/import.ts @@ -21,11 +21,12 @@ import { ImportJobContext, InjectedProps, OperationType, + ProductImportBatchJob, ProductImportCsvSchema, TBuiltProductImportLine, TParsedProductImportRowData, } from "./types" -import { SalesChannel } from "../../../models" +import { BatchJob, SalesChannel } from "../../../models" import { FlagRouter } from "../../../utils/flag-router" import { transformProductData, transformVariantData } from "./utils" import SalesChannelFeatureFlag from "../../../loaders/feature-flags/sales-channels" @@ -135,7 +136,10 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { async getImportInstructions( csvData: TParsedProductImportRowData[] ): Promise> { - const shippingProfile = await this.shippingProfileService_.retrieveDefault() + const transactionManager = this.transactionManager_ ?? this.manager_ + const shippingProfile = await this.shippingProfileService_ + .withTransaction(transactionManager) + .retrieveDefault() const seenProducts = {} @@ -224,43 +228,64 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { * @param batchJobId - An id of a job that is being preprocessed. */ async preProcessBatchJob(batchJobId: string): Promise { - const batchJob = await this.batchJobService_.retrieve(batchJobId) + const transactionManager = this.transactionManager_ ?? this.manager_ + const batchJob = await this.batchJobService_ + .withTransaction(transactionManager) + .retrieve(batchJobId) const csvFileKey = (batchJob.context as ImportJobContext).fileKey const csvStream = await this.fileService_.getDownloadStream({ fileKey: csvFileKey, }) - const parsedData = await this.csvParser_.parse(csvStream) - const builtData = await this.csvParser_.buildData(parsedData) + let builtData: Record[] + try { + const parsedData = await this.csvParser_.parse(csvStream) + builtData = await this.csvParser_.buildData(parsedData) + } catch (e) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + "The csv file parsing failed due to: " + e.message + ) + } const ops = await this.getImportInstructions(builtData) await this.uploadImportOpsFile(batchJobId, ops) - await this.batchJobService_.update(batchJobId, { - result: { - advancement_count: 0, - // number of update/create operations to execute - count: Object.keys(ops).reduce((acc, k) => acc + ops[k].length, 0), - stat_descriptors: [ - { - key: "product-import-count", - name: "Products/variants to import", - message: `There will be ${ - ops[OperationType.ProductCreate].length - } products created (${ - ops[OperationType.ProductUpdate].length - } updated). + let totalOperationCount = 0 + const operationsCounts = {} + Object.keys(ops).forEach((key) => { + operationsCounts[key] = ops[key].length + totalOperationCount += ops[key].length + }) + + await this.batchJobService_ + .withTransaction(transactionManager) + .update(batchJobId, { + result: { + advancement_count: 0, + // number of update/create operations to execute + count: totalOperationCount, + operations: operationsCounts, + stat_descriptors: [ + { + key: "product-import-count", + name: "Products/variants to import", + message: `There will be ${ + ops[OperationType.ProductCreate].length + } products created (${ + ops[OperationType.ProductUpdate].length + } updated). ${ ops[OperationType.VariantCreate].length } variants will be created and ${ - ops[OperationType.VariantUpdate].length - } updated`, - }, - ], - }, - }) + ops[OperationType.VariantUpdate].length + } updated`, + }, + ], + }, + }) } /** @@ -270,13 +295,17 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { * @param batchJobId - An id of a batch job that is being processed. */ async processJob(batchJobId: string): Promise { - return await this.atomicPhase_(async () => { - await this.createProducts(batchJobId) - await this.updateProducts(batchJobId) - await this.createVariants(batchJobId) - await this.updateVariants(batchJobId) + return await this.atomicPhase_(async (manager) => { + const batchJob = (await this.batchJobService_ + .withTransaction(manager) + .retrieve(batchJobId)) as ProductImportBatchJob - this.finalize(batchJobId) + await this.createProducts(batchJob) + await this.updateProducts(batchJob) + await this.createVariants(batchJob) + await this.updateVariants(batchJob) + + await this.finalize(batchJob) }) } @@ -326,12 +355,17 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { /** * Method creates products using `ProductService` and parsed data from a CSV row. * - * @param batchJobId - An id of the current batch job being processed. + * @param batchJob - The current batch job being processed. */ - private async createProducts(batchJobId: string): Promise { + private async createProducts(batchJob: ProductImportBatchJob): Promise { + if (!batchJob.result.operations[OperationType.ProductCreate]) { + return + } + const transactionManager = this.transactionManager_ ?? this.manager_ + const productOps = await this.downloadImportOpsFile( - batchJobId, + batchJob.id, OperationType.ProductCreate ) @@ -362,19 +396,23 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { ProductImportStrategy.throwDescriptiveError(productOp, e.message) } - this.updateProgress(batchJobId) + await this.updateProgress(batchJob.id) } } /** * Method updates existing products in the DB using a CSV row data. * - * @param batchJobId - An id of the current batch job being processed. + * @param batchJob - The current batch job being processed. */ - private async updateProducts(batchJobId: string): Promise { + private async updateProducts(batchJob: ProductImportBatchJob): Promise { + if (!batchJob.result.operations[OperationType.ProductUpdate]) { + return + } + const transactionManager = this.transactionManager_ ?? this.manager_ const productOps = await this.downloadImportOpsFile( - batchJobId, + batchJob.id, OperationType.ProductUpdate ) @@ -405,7 +443,7 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { ProductImportStrategy.throwDescriptiveError(productOp, e.message) } - this.updateProgress(batchJobId) + await this.updateProgress(batchJob.id) } } @@ -413,13 +451,17 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { * Method creates product variants from a CSV data. * Method also handles processing of variant options. * - * @param batchJobId - An id of the current batch job being processed. + * @param batchJob - The current batch job being processed. */ - private async createVariants(batchJobId: string): Promise { + private async createVariants(batchJob: ProductImportBatchJob): Promise { + if (!batchJob.result.operations[OperationType.VariantCreate]) { + return + } + const transactionManager = this.transactionManager_ ?? this.manager_ const variantOps = await this.downloadImportOpsFile( - batchJobId, + batchJob.id, OperationType.VariantCreate ) @@ -452,7 +494,7 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { .withTransaction(transactionManager) .create(product!, variant as unknown as CreateProductVariantInput) - this.updateProgress(batchJobId) + await this.updateProgress(batchJob.id) } catch (e) { ProductImportStrategy.throwDescriptiveError(variantOp, e.message) } @@ -462,13 +504,17 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { /** * Method updates product variants from a CSV data. * - * @param batchJobId - An id of the current batch job being processed. + * @param batchJob - The current batch job being processed. */ - private async updateVariants(batchJobId: string): Promise { + private async updateVariants(batchJob: ProductImportBatchJob): Promise { + if (!batchJob.result.operations[OperationType.VariantUpdate]) { + return + } + const transactionManager = this.transactionManager_ ?? this.manager_ const variantOps = await this.downloadImportOpsFile( - batchJobId, + batchJob.id, OperationType.VariantUpdate ) @@ -493,7 +539,7 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { ProductImportStrategy.throwDescriptiveError(variantOp, e.message) } - this.updateProgress(batchJobId) + await this.updateProgress(batchJob.id) } } @@ -507,10 +553,13 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { variantOp, productId: string ): Promise { + const transactionManager = this.transactionManager_ ?? this.manager_ const productOptions = variantOp["variant.options"] || [] + const productServiceTx = + this.productService_.withTransaction(transactionManager) for (const o of productOptions) { - const option = await this.productService_.retrieveOptionByTitle( + const option = await productServiceTx.retrieveOptionByTitle( o._title, productId ) @@ -536,7 +585,7 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { const { writeStream, promise } = await this.fileService_ .withTransaction(transactionManager) .getUploadStreamDescriptor({ - name: `imports/products/ops/${batchJobId}-${op}`, + name: ProductImportStrategy.buildFilename(batchJobId, op), ext: "json", }) @@ -566,7 +615,9 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { const readableStream = await this.fileService_ .withTransaction(transactionManager) .getDownloadStream({ - fileKey: `imports/products/ops/${batchJobId}-${op}.json`, + fileKey: ProductImportStrategy.buildFilename(batchJobId, op, { + appendExt: ".json", + }), }) return await new Promise((resolve) => { @@ -591,10 +642,13 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { protected async deleteOpsFiles(batchJobId: string): Promise { const transactionManager = this.transactionManager_ ?? this.manager_ - for (const op of Object.keys(OperationType)) { + const fileServiceTx = this.fileService_.withTransaction(transactionManager) + for (const op of Object.values(OperationType)) { try { - this.fileService_.withTransaction(transactionManager).delete({ - fileKey: `imports/products/ops/-${batchJobId}-${op}`, + await fileServiceTx.delete({ + fileKey: ProductImportStrategy.buildFilename(batchJobId, op, { + appendExt: ".json", + }), }) } catch (e) { // noop @@ -606,22 +660,26 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { * Update count of processed data in the batch job `result` column * and cleanup temp JSON files. * - * @param batchJobId - An id of the current batch job being processed. + * @param batchJob - The current batch job being processed. */ - private async finalize(batchJobId: string): Promise { - const batchJob = await this.batchJobService_.retrieve(batchJobId) + private async finalize(batchJob: BatchJob): Promise { + const transactionManager = this.transactionManager_ ?? this.manager_ - delete this.processedCounter[batchJobId] + delete this.processedCounter[batchJob.id] - await this.batchJobService_.update(batchJobId, { - result: { advancement_count: batchJob.result.count }, - }) + await this.batchJobService_ + .withTransaction(transactionManager) + .update(batchJob.id, { + result: { advancement_count: batchJob.result.count }, + }) const { fileKey } = batchJob.context as ImportJobContext - await this.fileService_.delete({ fileKey }) + await this.fileService_ + .withTransaction(transactionManager) + .delete({ fileKey }) - await this.deleteOpsFiles(batchJobId) + await this.deleteOpsFiles(batchJob.id) } /** @@ -639,11 +697,22 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { return } - await this.batchJobService_.update(batchJobId, { - result: { - advancement_count: newCount, - }, - }) + await this.batchJobService_ + .withTransaction(this.transactionManager_ ?? this.manager_) + .update(batchJobId, { + result: { + advancement_count: newCount, + }, + }) + } + + private static buildFilename( + batchJobId: string, + operation: string, + { appendExt }: { appendExt?: string } = { appendExt: undefined } + ): string { + const filename = `imports/products/ops/${batchJobId}-${operation}` + return appendExt ? filename + appendExt : filename } } @@ -675,7 +744,7 @@ const CSVSchema: ProductImportCsvSchema = { { name: "Product Height", mapTo: "product.height" }, { name: "Product HS Code", mapTo: "product.hs_code" }, { name: "Product Origin Country", mapTo: "product.origin_country" }, - { name: "Product Mid Code", mapTo: "product.mid_code" }, + { name: "Product MID Code", mapTo: "product.mid_code" }, { name: "Product Material", mapTo: "product.material" }, // PRODUCT-COLLECTION { name: "Product Collection Title", mapTo: "product.collection.title" }, @@ -712,7 +781,7 @@ const CSVSchema: ProductImportCsvSchema = { { name: "Variant Height", mapTo: "variant.height" }, { name: "Variant HS Code", mapTo: "variant.hs_code" }, { name: "Variant Origin Country", mapTo: "variant.origin_country" }, - { name: "Variant Mid Code", mapTo: "variant.mid_code" }, + { name: "Variant MID Code", mapTo: "variant.mid_code" }, { name: "Variant Material", mapTo: "variant.material" }, // ==== DYNAMIC FIELDS ==== @@ -776,7 +845,6 @@ const CSVSchema: ProductImportCsvSchema = { } const regionName = key.split(" ")[1] - ;( builtLine["variant.prices"] as Record[] ).push({ @@ -802,7 +870,6 @@ const CSVSchema: ProductImportCsvSchema = { } const currency = key.split(" ")[1] - ;( builtLine["variant.prices"] as Record[] ).push({ diff --git a/packages/medusa/src/strategies/batch-jobs/product/index.ts b/packages/medusa/src/strategies/batch-jobs/product/index.ts index 4bad5a93e8..28bcdacf2d 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/index.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/index.ts @@ -52,7 +52,7 @@ export const productExportSchemaDescriptors = new Map< ProductExportColumnSchemaDescriptor >([ [ - "Product ID", + "Product id", { accessor: (product: Product): string => product?.id ?? "", entityName: "product", @@ -219,7 +219,7 @@ export const productExportSchemaDescriptors = new Map< }, ], [ - "Variant ID", + "Variant id", { accessor: (variant: ProductVariant): string => variant?.id ?? "", entityName: "variant", diff --git a/packages/medusa/src/strategies/batch-jobs/product/types.ts b/packages/medusa/src/strategies/batch-jobs/product/types.ts index 2a43516691..f72fa597d9 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/types.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/types.ts @@ -11,6 +11,15 @@ import { } from "../../../services" import { CsvSchema } from "../../../interfaces/csv-parser" import { FlagRouter } from "../../../utils/flag-router" +import { BatchJob } from "../../../models" + +export type ProductImportBatchJob = BatchJob & { + result: Pick & { + operations: { + [K in keyof typeof OperationType]: number + } + } +} /** * DI props for the Product import strategy diff --git a/packages/medusa/src/strategies/batch-jobs/product/utils.ts b/packages/medusa/src/strategies/batch-jobs/product/utils.ts index 67826d619c..5e4f7459fc 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/utils.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/utils.ts @@ -1,10 +1,11 @@ +import { TParsedProductImportRowData } from "./types" +import { csvRevertCellContentFormatter } from "../../../utils" + /** * Pick keys for a new object by regex. * @param data - Initial data object * @param regex - A regex used to pick which keys are going to be copied in the new object */ -import { TParsedProductImportRowData } from "./types" - export function pickObjectPropsByRegex( data: TParsedProductImportRowData, regex: RegExp @@ -14,7 +15,11 @@ export function pickObjectPropsByRegex( for (const k in data) { if (variantKeyPredicate(k)) { - ret[k] = data[k] + const formattedData = + typeof data[k] === "string" + ? csvRevertCellContentFormatter(data[k] as string) + : data[k] + ret[k] = formattedData } } diff --git a/packages/medusa/src/subscribers/batch-job.ts b/packages/medusa/src/subscribers/batch-job.ts index 8dcc407cd6..f362b194b6 100644 --- a/packages/medusa/src/subscribers/batch-job.ts +++ b/packages/medusa/src/subscribers/batch-job.ts @@ -1,26 +1,31 @@ import BatchJobService from "../services/batch-job" import EventBusService from "../services/event-bus" import { StrategyResolverService } from "../services" +import { EntityManager } from "typeorm" type InjectedDependencies = { eventBusService: EventBusService batchJobService: BatchJobService strategyResolverService: StrategyResolverService + manager: EntityManager } class BatchJobSubscriber { private readonly eventBusService_: EventBusService private readonly batchJobService_: BatchJobService private readonly strategyResolver_: StrategyResolverService + private readonly manager_: EntityManager constructor({ eventBusService, batchJobService, strategyResolverService, + manager, }: InjectedDependencies) { this.eventBusService_ = eventBusService this.batchJobService_ = batchJobService this.strategyResolver_ = strategyResolverService + this.manager_ = manager this.eventBusService_ .subscribe(BatchJobService.Events.CREATED, this.preProcessBatchJob) @@ -28,37 +33,45 @@ class BatchJobSubscriber { } preProcessBatchJob = async (data): Promise => { - const batchJob = await this.batchJobService_.retrieve(data.id) + await this.manager_.transaction(async (manager) => { + const batchJobServiceTx = this.batchJobService_.withTransaction(manager) + const batchJob = await batchJobServiceTx.retrieve(data.id) - const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType( - batchJob.type - ) + const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType( + batchJob.type + ) - try { - await batchJobStrategy.preProcessBatchJob(batchJob.id) - await this.batchJobService_.setPreProcessingDone(batchJob.id) - } catch (e) { - await this.batchJobService_.setFailed(batchJob.id) - throw e - } + try { + await batchJobStrategy + .withTransaction(manager) + .preProcessBatchJob(batchJob.id) + await batchJobServiceTx.setPreProcessingDone(batchJob.id) + } catch (e) { + await this.batchJobService_.setFailed(batchJob.id) + throw e + } + }) } processBatchJob = async (data): Promise => { - const batchJob = await this.batchJobService_.retrieve(data.id) + await this.manager_.transaction(async (manager) => { + const batchJobServiceTx = this.batchJobService_.withTransaction(manager) + const batchJob = await batchJobServiceTx.retrieve(data.id) - const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType( - batchJob.type - ) + const batchJobStrategy = this.strategyResolver_.resolveBatchJobByType( + batchJob.type + ) - await this.batchJobService_.setProcessing(batchJob.id) + await batchJobServiceTx.setProcessing(batchJob.id) - try { - await batchJobStrategy.processJob(batchJob.id) - await this.batchJobService_.complete(batchJob.id) - } catch (e) { - await this.batchJobService_.setFailed(batchJob.id) - throw e - } + try { + await batchJobStrategy.withTransaction(manager).processJob(batchJob.id) + await batchJobServiceTx.complete(batchJob.id) + } catch (e) { + await this.batchJobService_.setFailed(batchJob.id) + throw e + } + }) } } diff --git a/packages/medusa/src/utils/__tests__/csv-cell-content-formatter.spec.ts b/packages/medusa/src/utils/__tests__/csv-cell-content-formatter.spec.ts new file mode 100644 index 0000000000..888fae37ce --- /dev/null +++ b/packages/medusa/src/utils/__tests__/csv-cell-content-formatter.spec.ts @@ -0,0 +1,43 @@ +import { csvCellContentFormatter } from "../csv-cell-content-formatter" + +type Case = { + str: string + expected: string +} + +const cases: [string, Case][] = [ + [ + "should return the exact input when there is no new line char", + { + str: "Hello, my name is Adrien and I like writing single line content.", + expected: + "Hello, my name is Adrien and I like writing single line content.", + }, + ], + [ + "should return a formatted string escaping new line when there is new line chars", + { + str: `Hello, +my name is Adrien and +I like writing multiline content +in a template string`, + expected: + '"Hello,\nmy name is Adrien and\nI like writing multiline content\nin a template string"', + }, + ], + [ + "should return a formatted string escaping new line when there is new line chars and escape the double quote when there is double quotes", + { + str: 'Hello,\nmy name is "Adrien" and\nI like writing multiline content\nin a string', + expected: + '"Hello,\nmy name is ""Adrien"" and\nI like writing multiline content\nin a string"', + }, + ], +] + +describe("csvCellContentFormatter", function () { + it.each(cases)("%s", (title: string, { str, expected }: Case) => { + const formattedStr = csvCellContentFormatter(str) + expect(formattedStr).toBe(expected) + }) +}) diff --git a/packages/medusa/src/utils/csv-cell-content-formatter.ts b/packages/medusa/src/utils/csv-cell-content-formatter.ts new file mode 100644 index 0000000000..fab2a01c90 --- /dev/null +++ b/packages/medusa/src/utils/csv-cell-content-formatter.ts @@ -0,0 +1,20 @@ +export function csvCellContentFormatter(str: string): string { + const newLineRegexp = new RegExp(/\n/g) + const doubleQuoteRegexp = new RegExp(/"/g) + + const hasNewLineChar = !!str.match(newLineRegexp) + if (!hasNewLineChar) { + return str + } + + const formatterStr = str.replace(doubleQuoteRegexp, '""') + + return `"${formatterStr}"` +} + +export function csvRevertCellContentFormatter(str: string): string { + if (str.startsWith('"')) { + str = str.substring(1, str.length - 1) + } + return str +} diff --git a/packages/medusa/src/utils/index.ts b/packages/medusa/src/utils/index.ts index e353050941..7fd1525d33 100644 --- a/packages/medusa/src/utils/index.ts +++ b/packages/medusa/src/utils/index.ts @@ -5,3 +5,4 @@ export * from "./generate-entity-id" export * from "./remove-undefined-properties" export * from "./is-defined" export * from "./calculate-price-tax-amount" +export * from "./csv-cell-content-formatter"