From 9de1d8c9c37506fad3760af260c8d27570b2d3ed Mon Sep 17 00:00:00 2001 From: Stevche Radevski Date: Tue, 30 Jul 2024 23:14:33 +0300 Subject: [PATCH] fix: Added more tests and fixed a couple of issues with product import (#8341) --- .../admin/__fixtures__/invalid-column.csv | 2 + .../admin/__fixtures__/invalid-prices.csv | 2 + .../admin/__fixtures__/unrelated-column.csv | 2 + .../product/admin/product-import.spec.ts | 119 +++++++++++++++++- .../src/notification/steps/index.ts | 1 + .../notification/steps/notify-on-failure.ts | 35 ++++++ .../product/helpers/normalize-for-import.ts | 22 +++- .../src/product/steps/parse-product-csv.ts | 19 ++- .../src/product/workflows/import-products.ts | 21 +++- 9 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 integration-tests/http/__tests__/product/admin/__fixtures__/invalid-column.csv create mode 100644 integration-tests/http/__tests__/product/admin/__fixtures__/invalid-prices.csv create mode 100644 integration-tests/http/__tests__/product/admin/__fixtures__/unrelated-column.csv create mode 100644 packages/core/core-flows/src/notification/steps/notify-on-failure.ts diff --git a/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-column.csv b/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-column.csv new file mode 100644 index 0000000000..510649018b --- /dev/null +++ b/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-column.csv @@ -0,0 +1,2 @@ +Product Id,Product Title,Product Subtitle,Product Status,Product External Id,Product Description,Product Handle,Product Is Giftcard,Product Discountable,Product Thumbnail,Product Collection Id,Product Type Id,Product Weight,Product Length,Product Height,Product Width,Product Hs Code,Product Origin Country,Product Mid Code,Product Material,Product Created At,Product Updated At,Product Deleted At,Product Image 1,Product Image 2,Product Tag 1,Variant Id,Variant Title,Variant Sku,Variant Barcode,Variant Ean,Variant Upc,Variant Allow Backorder,Variant Manage Inventory,Variant Hs Code,Variant Origin Country,Variant Mid Code,Variant Material,Variant Weight,Variant Length,Variant Height,Variant Width,Variant Metadata,Variant Variant Rank,Variant Product Id,Variant Created At,Variant Updated At,Variant Deleted At,Variant Price USD,Variant Price EUR,Variant Price DKK,Variant Option 1 Name,Variant Option 1 Value,Variant Option 2 Name,Variant Option 2 Value,Product field +prod_01J3CSN791SN1RN7X155Z8S9CN,Proposed product,,proposed,,test-product-description,proposed-product,false,true,test-image.png,,ptyp_01J3CSN76GCRSCDV9V489B5FWQ,,,,,,,,,2024-07-22T08:41:47.040Z,2024-07-22T08:41:47.040Z,,test-image.png,test-image-2.png,new-tag,variant_01J3CSN79CQ2ND94SRJSXMEMNH,Test variant,,,,,false,true,,,,,,,,,,0,prod_01J3CSN791SN1RN7X155Z8S9CN,2024-07-22T08:41:47.053Z,2024-07-22T08:41:47.053Z,,100,45,30,size,large,color,green,someval \ No newline at end of file diff --git a/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-prices.csv b/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-prices.csv new file mode 100644 index 0000000000..4239fe9065 --- /dev/null +++ b/integration-tests/http/__tests__/product/admin/__fixtures__/invalid-prices.csv @@ -0,0 +1,2 @@ +Product Id,Product Title,Product Subtitle,Product Status,Product External Id,Product Description,Product Handle,Product Is Giftcard,Product Discountable,Product Thumbnail,Product Collection Id,Product Type Id,Product Weight,Product Length,Product Height,Product Width,Product Hs Code,Product Origin Country,Product Mid Code,Product Material,Product Created At,Product Updated At,Product Deleted At,Product Image 1,Product Image 2,Product Tag 1,Variant Id,Variant Title,Variant Sku,Variant Barcode,Variant Ean,Variant Upc,Variant Allow Backorder,Variant Manage Inventory,Variant Hs Code,Variant Origin Country,Variant Mid Code,Variant Material,Variant Weight,Variant Length,Variant Height,Variant Width,Variant Metadata,Variant Variant Rank,Variant Product Id,Variant Created At,Variant Updated At,Variant Deleted At,Variant Price USD,Variant Price EUR,Variant Price reg_nonexistent,Variant Option 1 Name,Variant Option 1 Value,Variant Option 2 Name,Variant Option 2 Value +prod_01J3CSN791SN1RN7X155Z8S9CN,Proposed product,,proposed,,test-product-description,proposed-product,false,true,test-image.png,,ptyp_01J3CSN76GCRSCDV9V489B5FWQ,,,,,,,,,2024-07-22T08:41:47.040Z,2024-07-22T08:41:47.040Z,,test-image.png,test-image-2.png,new-tag,variant_01J3CSN79CQ2ND94SRJSXMEMNH,Test variant,,,,,false,true,,,,,,,,,,0,prod_01J3CSN791SN1RN7X155Z8S9CN,2024-07-22T08:41:47.053Z,2024-07-22T08:41:47.053Z,,100,45,30,size,large,color,green \ No newline at end of file diff --git a/integration-tests/http/__tests__/product/admin/__fixtures__/unrelated-column.csv b/integration-tests/http/__tests__/product/admin/__fixtures__/unrelated-column.csv new file mode 100644 index 0000000000..ce999700e7 --- /dev/null +++ b/integration-tests/http/__tests__/product/admin/__fixtures__/unrelated-column.csv @@ -0,0 +1,2 @@ +Product Id,Product Title,Product Subtitle,Product Status,Product External Id,Product Description,Product Handle,Product Is Giftcard,Product Discountable,Product Thumbnail,Product Collection Id,Product Type Id,Product Weight,Product Length,Product Height,Product Width,Product Hs Code,Product Origin Country,Product Mid Code,Product Material,Product Created At,Product Updated At,Product Deleted At,Product Image 1,Product Image 2,Product Tag 1,Variant Id,Variant Title,Variant Sku,Variant Barcode,Variant Ean,Variant Upc,Variant Allow Backorder,Variant Manage Inventory,Variant Hs Code,Variant Origin Country,Variant Mid Code,Variant Material,Variant Weight,Variant Length,Variant Height,Variant Width,Variant Metadata,Variant Variant Rank,Variant Product Id,Variant Created At,Variant Updated At,Variant Deleted At,Variant Price USD,Variant Price EUR,Variant Price DKK,Variant Option 1 Name,Variant Option 1 Value,Variant Option 2 Name,Variant Option 2 Value,Some field +prod_01J3CSN791SN1RN7X155Z8S9CN,Proposed product,,proposed,,test-product-description,proposed-product,false,true,test-image.png,,ptyp_01J3CSN76GCRSCDV9V489B5FWQ,,,,,,,,,2024-07-22T08:41:47.040Z,2024-07-22T08:41:47.040Z,,test-image.png,test-image-2.png,new-tag,variant_01J3CSN79CQ2ND94SRJSXMEMNH,Test variant,,,,,false,true,,,,,,,,,,0,prod_01J3CSN791SN1RN7X155Z8S9CN,2024-07-22T08:41:47.053Z,2024-07-22T08:41:47.053Z,,100,45,30,size,large,color,green,someval \ No newline at end of file diff --git a/integration-tests/http/__tests__/product/admin/product-import.spec.ts b/integration-tests/http/__tests__/product/admin/product-import.spec.ts index 5fc1723c92..829b331fff 100644 --- a/integration-tests/http/__tests__/product/admin/product-import.spec.ts +++ b/integration-tests/http/__tests__/product/admin/product-import.spec.ts @@ -322,8 +322,123 @@ medusaIntegrationTestRunner({ ) }) - it("should fail on invalid prices being present in the CSV", async () => {}) - it("should fail on non-existent fields being present in the CSV", async () => {}) + it("should fail on invalid region in prices being present in the CSV", async () => { + let fileContent = await fs.readFile( + path.join(__dirname, "__fixtures__", "invalid-prices.csv"), + { encoding: "utf-8" } + ) + + const { form, meta } = getUploadReq({ + name: "test.csv", + content: fileContent, + }) + + const err = await api + .post("/admin/products/import", form, meta) + .catch((e) => e) + expect(err.response.data.message).toEqual( + "Region with ID reg_nonexistent not found" + ) + }) + + it("should ignore non-existent fields being present in the CSV that don't start with Product or Variant", async () => { + const subscriberExecution = TestEventUtils.waitSubscribersExecution( + "notification.notification.created", + eventBus + ) + + let fileContent = await fs.readFile( + path.join(__dirname, "__fixtures__", "unrelated-column.csv"), + { encoding: "utf-8" } + ) + + fileContent = fileContent.replace(/pcol_\w*\d*/g, baseCollection.id) + fileContent = fileContent.replace(/ptyp_\w*\d*/g, baseType.id) + + const { form, meta } = getUploadReq({ + name: "test.csv", + content: fileContent, + }) + + const batchJobRes = await api.post("/admin/products/import", form, meta) + + const transactionId = batchJobRes.data.transaction_id + expect(transactionId).toBeTruthy() + expect(batchJobRes.data.summary).toEqual({ + toCreate: 1, + toUpdate: 0, + }) + + await api.post( + `/admin/products/import/${transactionId}/confirm`, + {}, + meta + ) + + await subscriberExecution + const notifications = ( + await api.get("/admin/notifications", adminHeaders) + ).data.notifications + + expect(notifications.length).toBe(1) + expect(notifications[0]).toEqual( + expect.objectContaining({ + data: expect.objectContaining({ + title: "Product import", + description: `Product import of file test.csv completed successfully!`, + }), + }) + ) + }) + + it("should fail on non-existent product fields being present in the CSV", async () => { + const subscriberExecution = TestEventUtils.waitSubscribersExecution( + "notification.notification.created", + eventBus + ) + + let fileContent = await fs.readFile( + path.join(__dirname, "__fixtures__", "invalid-column.csv"), + { encoding: "utf-8" } + ) + + fileContent = fileContent.replace(/pcol_\w*\d*/g, baseCollection.id) + fileContent = fileContent.replace(/ptyp_\w*\d*/g, baseType.id) + + const { form, meta } = getUploadReq({ + name: "test.csv", + content: fileContent, + }) + + const batchJobRes = await api.post("/admin/products/import", form, meta) + + const transactionId = batchJobRes.data.transaction_id + expect(transactionId).toBeTruthy() + expect(batchJobRes.data.summary).toEqual({ + toCreate: 1, + toUpdate: 0, + }) + + await api + .post(`/admin/products/import/${transactionId}/confirm`, {}, meta) + // TODO: Currently the `setStepSuccess` waits for the whole workflow to finish before returning. + .catch((e) => e) + + await subscriberExecution + const notifications = ( + await api.get("/admin/notifications", adminHeaders) + ).data.notifications + + expect(notifications.length).toBe(1) + expect(notifications[0]).toEqual( + expect.objectContaining({ + data: expect.objectContaining({ + title: "Product import", + description: `Failed to import products from file test.csv`, + }), + }) + ) + }) }) }, }) diff --git a/packages/core/core-flows/src/notification/steps/index.ts b/packages/core/core-flows/src/notification/steps/index.ts index 9a0460ae9e..364df34bfc 100644 --- a/packages/core/core-flows/src/notification/steps/index.ts +++ b/packages/core/core-flows/src/notification/steps/index.ts @@ -1 +1,2 @@ export * from "./send-notifications" +export * from "./notify-on-failure" diff --git a/packages/core/core-flows/src/notification/steps/notify-on-failure.ts b/packages/core/core-flows/src/notification/steps/notify-on-failure.ts new file mode 100644 index 0000000000..d1f0e6547a --- /dev/null +++ b/packages/core/core-flows/src/notification/steps/notify-on-failure.ts @@ -0,0 +1,35 @@ +import { INotificationModuleService } from "@medusajs/types" +import { ModuleRegistrationName } from "@medusajs/utils" +import { StepResponse, createStep } from "@medusajs/workflows-sdk" + +type NotifyOnFailureStepInput = { + to: string + channel: string + template: string + data?: Record | null + trigger_type?: string | null + resource_id?: string | null + resource_type?: string | null + receiver_id?: string | null + original_notification_id?: string | null + idempotency_key?: string | null +}[] + +export const notifyOnFailureStepId = "notify-on-failure" +export const notifyOnFailureStep = createStep( + notifyOnFailureStepId, + async (data: NotifyOnFailureStepInput) => { + return new StepResponse(void 0, data) + }, + + async (data, { container }) => { + if (!data) { + return + } + + const service = container.resolve( + ModuleRegistrationName.NOTIFICATION + ) + await service.createNotifications(data) + } +) diff --git a/packages/core/core-flows/src/product/helpers/normalize-for-import.ts b/packages/core/core-flows/src/product/helpers/normalize-for-import.ts index 99d4020163..adf13b26a5 100644 --- a/packages/core/core-flows/src/product/helpers/normalize-for-import.ts +++ b/packages/core/core-flows/src/product/helpers/normalize-for-import.ts @@ -1,9 +1,10 @@ -import { HttpTypes } from "@medusajs/types" +import { HttpTypes, RegionTypes } from "@medusajs/types" import { MedusaError, lowerCaseFirst } from "@medusajs/utils" // We want to convert the csv data format to a standard DTO format. export const normalizeForImport = ( - rawProducts: object[] + rawProducts: object[], + regions: RegionTypes.RegionDTO[] ): HttpTypes.AdminCreateProduct[] => { const productMap = new Map< string, @@ -12,13 +13,14 @@ export const normalizeForImport = ( variants: HttpTypes.AdminCreateProductVariant[] } >() + const regionsMap = new Map(regions.map((r) => [r.id, r])) rawProducts.forEach((rawProduct) => { const productInMap = productMap.get(rawProduct["Product Handle"]) if (!productInMap) { productMap.set(rawProduct["Product Handle"], { product: normalizeProductForImport(rawProduct), - variants: [normalizeVariantForImport(rawProduct)], + variants: [normalizeVariantForImport(rawProduct, regionsMap)], }) return } @@ -27,7 +29,7 @@ export const normalizeForImport = ( product: productInMap.product, variants: [ ...productInMap.variants, - normalizeVariantForImport(rawProduct), + normalizeVariantForImport(rawProduct, regionsMap), ], }) }) @@ -125,7 +127,8 @@ const normalizeProductForImport = ( } const normalizeVariantForImport = ( - rawProduct: object + rawProduct: object, + regionsMap: Map ): HttpTypes.AdminCreateProductVariant => { const response = {} const options = new Map() @@ -148,10 +151,19 @@ const normalizeVariantForImport = ( { currency_code: priceKey.toLowerCase(), amount: normalizedValue }, ] } else { + const region = regionsMap.get(priceKey) + if (!region) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Region with ID ${priceKey} not found` + ) + } + response["prices"] = [ ...(response["prices"] || []), { amount: normalizedValue, + currency_code: region.currency_code, rules: { region_id: priceKey }, }, ] diff --git a/packages/core/core-flows/src/product/steps/parse-product-csv.ts b/packages/core/core-flows/src/product/steps/parse-product-csv.ts index 4bee5227ac..0bbc1dbb70 100644 --- a/packages/core/core-flows/src/product/steps/parse-product-csv.ts +++ b/packages/core/core-flows/src/product/steps/parse-product-csv.ts @@ -1,11 +1,19 @@ -import { MedusaError, convertCsvToJson } from "@medusajs/utils" +import { + MedusaError, + ModuleRegistrationName, + convertCsvToJson, +} from "@medusajs/utils" import { StepResponse, createStep } from "@medusajs/workflows-sdk" import { normalizeForImport } from "../helpers/normalize-for-import" +import { IRegionModuleService } from "@medusajs/types" export const parseProductCsvStepId = "parse-product-csv" export const parseProductCsvStep = createStep( parseProductCsvStepId, - async (fileContent: string) => { + async (fileContent: string, { container }) => { + const regionService = container.resolve( + ModuleRegistrationName.REGION + ) const csvProducts = convertCsvToJson(fileContent) csvProducts.forEach((product: any) => { @@ -17,7 +25,12 @@ export const parseProductCsvStep = createStep( } }) - const normalizedData = normalizeForImport(csvProducts) + const allRegions = await regionService.listRegions( + {}, + { select: ["id", "currency_code"], take: null } + ) + + const normalizedData = normalizeForImport(csvProducts, allRegions) return new StepResponse(normalizedData) } ) diff --git a/packages/core/core-flows/src/product/workflows/import-products.ts b/packages/core/core-flows/src/product/workflows/import-products.ts index c0b8ffec12..d9bfc43808 100644 --- a/packages/core/core-flows/src/product/workflows/import-products.ts +++ b/packages/core/core-flows/src/product/workflows/import-products.ts @@ -4,7 +4,7 @@ import { transform, } from "@medusajs/workflows-sdk" import { WorkflowTypes } from "@medusajs/types" -import { sendNotificationsStep } from "../../notification" +import { notifyOnFailureStep, sendNotificationsStep } from "../../notification" import { waitConfirmationProductImportStep, groupProductsForBatchStep, @@ -30,6 +30,23 @@ export const importProductsWorkflow = createWorkflow( waitConfirmationProductImportStep() + // Q: Can we somehow access the error from the step that threw here? Or in a compensate step at least? + const failureNotification = transform({ input }, (data) => { + return [ + { + // We don't need the recipient here for now, but if we want to push feed notifications to a specific user we could add it. + to: "", + channel: "feed", + template: "admin-ui", + data: { + title: "Product import", + description: `Failed to import products from file ${data.input.filename}`, + }, + }, + ] + }) + notifyOnFailureStep(failureNotification) + batchProductsWorkflow.runAsStep({ input: batchRequest }) const notifications = transform({ input }, (data) => { @@ -46,8 +63,8 @@ export const importProductsWorkflow = createWorkflow( }, ] }) - sendNotificationsStep(notifications) + return summary } )