diff --git a/.changeset/six-mails-join.md b/.changeset/six-mails-join.md new file mode 100644 index 00000000..1ef45c38 --- /dev/null +++ b/.changeset/six-mails-join.md @@ -0,0 +1,12 @@ +--- +"@comet/brevo-admin": patch +"@comet/brevo-api": patch +--- + +CSV Import Validation Improvements and Bug Fix + +Add better validation for csv imports. + +Add better feedback after a csv import when something goes wrong. User can download a file with failing rows. + +Fix a bug when importing via csv in a targetgroup. The contact was only added to the manually assigned contacts and not to the actual target group. diff --git a/packages/admin/src/common/contactImport/useContactImportFromCsv.tsx b/packages/admin/src/common/contactImport/useContactImportFromCsv.tsx index 2ad9d157..eed88ff8 100644 --- a/packages/admin/src/common/contactImport/useContactImportFromCsv.tsx +++ b/packages/admin/src/common/contactImport/useContactImportFromCsv.tsx @@ -2,10 +2,12 @@ import { useApolloClient } from "@apollo/client"; import { RefetchQueriesInclude } from "@apollo/client/core/types"; import { Alert, Loading, messages, useErrorDialog } from "@comet/admin"; import { Upload } from "@comet/admin-icons"; -import { Box, Button, Dialog, DialogActions, DialogContent, DialogTitle } from "@mui/material"; +import { Box, Dialog, DialogActions, DialogContent, DialogTitle, styled } from "@mui/material"; +import Button from "@mui/material/Button"; +import saveAs from "file-saver"; import * as React from "react"; import { useDropzone } from "react-dropzone"; -import { FormattedMessage } from "react-intl"; +import { FormattedMessage, useIntl } from "react-intl"; import { GQLEmailCampaignContentScopeInput } from "../../graphql.generated"; import { CrudMoreActionsItem } from "../../temp/CrudMoreActionsMenu"; @@ -49,13 +51,22 @@ interface ComponentProps extends UseContactImportProps { fileInputRef: React.RefObject; } +interface ImportInformation { + failed: number; + created: number; + updated: number; + failedColumns: Record[]; + errorMessage?: string; +} + const ContactImportComponent = ({ scope, targetGroupId, fileInputRef, refetchQueries }: ComponentProps) => { const apolloClient = useApolloClient(); const [importingCsv, setImportingCsv] = React.useState(false); - const [importSuccessful, setImportSuccessful] = React.useState(false); - const dialogOpen = importingCsv || importSuccessful; + const [importInformation, setImportInformation] = React.useState(null); + const dialogOpen = importingCsv || !!importInformation; const errorDialog = useErrorDialog(); const config = useBrevoConfig(); + const intl = useIntl(); function upload(file: File, scope: GQLEmailCampaignContentScopeInput, listIds?: string[]): Promise { const formData = new FormData(); @@ -70,6 +81,34 @@ const ContactImportComponent = ({ scope, targetGroupId, fileInputRef, refetchQue }); } + const saveErrorFile = () => { + const failedColumns = importInformation?.failedColumns; + if (!failedColumns || failedColumns.length === 0) { + throw new Error(intl.formatMessage({ id: "export", defaultMessage: "No failed columns to save" })); + } + + let errorData = ""; + + // Add headers to the file without trailing semicolon + const headers = Object.keys(failedColumns[0]); + const headerStr = headers.join(";"); + errorData = `${headerStr.replace(/;+$/, "")}\n`; // Remove trailing semicolon from the header + + // Add each row of failed columns data + for (const column of failedColumns) { + // Use Object.values to get the values of each column + const row = Object.values(column); // No need to check for undefined/null + + // Join row values and remove the trailing semicolon if not needed + const rowStr = row.join(";"); + errorData += `${rowStr.replace(/;+$/, "")}\n`; + } + + // Create and download the file + const file = new Blob([errorData], { type: "text/csv;charset=utf-8" }); + saveAs(file, `error-log-${new Date().toISOString()}.csv`); + }; + const { getInputProps } = useDropzone({ accept: { "text/csv": [] }, multiple: false, @@ -81,30 +120,32 @@ const ContactImportComponent = ({ scope, targetGroupId, fileInputRef, refetchQue const response = await upload(file, scope, targetGroupId ? [targetGroupId] : []); apolloClient.refetchQueries({ include: refetchQueries }); + const data = (await response.json()) as ImportInformation; + if (response.ok) { setImportingCsv(false); - setImportSuccessful(true); + + if (data.errorMessage) { + errorDialog?.showError({ + title: , + userMessage: data.errorMessage, + error: data.errorMessage, + }); + } else { + setImportInformation(data); + } } else { - const errorResponse = await response.json(); - throw new Error(JSON.stringify(errorResponse)); + throw new Error(JSON.stringify(data)); } } catch (e) { setImportingCsv(false); - let userMessage = ( + const userMessage = ( ); - if (e?.message && typeof e.message === "string" && e.message.includes("Too many contacts")) { - userMessage = ( - - ); - } errorDialog?.showError({ title: , @@ -123,32 +164,62 @@ const ContactImportComponent = ({ scope, targetGroupId, fileInputRef, refetchQue {importingCsv && ( )} - {importSuccessful && ( + {importInformation && ( )} {importingCsv && } - {importSuccessful && ( + {importInformation && ( <> - - - - - - + {importInformation.created > 0 && ( + + )} + {importInformation.updated > 0 && ( + + )} + + {importInformation.failed > 0 && ( + + + ( + {chunks} + ), + }} + /> + + + )} + + {(importInformation.created > 0 || importInformation.updated > 0) && ( + + + + + + )} )} - {importSuccessful && ( - )} @@ -157,3 +228,9 @@ const ContactImportComponent = ({ scope, targetGroupId, fileInputRef, refetchQue ); }; + +const CsvDownloadLink = styled("span")` + color: ${({ theme }) => theme.palette.info.main}; + text-decoration: underline; + cursor: pointer; +`; diff --git a/packages/api/package.json b/packages/api/package.json index e59d8531..589097eb 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -51,6 +51,7 @@ "@nestjs/graphql": "^10.0.0", "@nestjs/platform-express": "^9.0.0", "@types/jest": "^29.5.0", + "@types/lodash.isequal": "^4.0.0", "@types/mime-db": "^1.43.5", "@types/node-fetch": "^2.5.12", "@types/rimraf": "^3.0.0", @@ -64,6 +65,7 @@ "graphql": "^15.5.0", "jest": "^29.5.0", "jest-junit": "^15.0.0", + "lodash.isequal": "^4.5.0", "nestjs-console": "^8.0.0", "pg-error-constants": "^1.0.0", "prettier": "^2.0.0", diff --git a/packages/api/src/brevo-contact/brevo-contact-import.console.ts b/packages/api/src/brevo-contact/brevo-contact-import.console.ts index 6451c779..3b3829b5 100644 --- a/packages/api/src/brevo-contact/brevo-contact-import.console.ts +++ b/packages/api/src/brevo-contact/brevo-contact-import.console.ts @@ -81,21 +81,19 @@ export function createBrevoContactImportConsole({ Scope }: { Scope: Type { - const redirectUrl = this.config.brevo.resolveConfig(options.scope).redirectUrlForImport; - const content = fs.readFileSync(options.path); - if (!this.validateRedirectUrl(redirectUrl, options.scope)) { + async execute({ scope, path, targetGroupIds }: CommandOptions): Promise { + const redirectUrl = this.config.brevo.resolveConfig(scope).redirectUrlForImport; + const fileStream = fs.createReadStream(path); + if (!this.validateRedirectUrl(redirectUrl, scope)) { throw new InvalidOptionArgumentError("Invalid scope. Scope is not allowed"); } - const targetGroups = await this.targetGroupRepository.find({ id: { $in: options.targetGroupIds } }); - - const result = await this.brevoContactImportService.importContactsFromCsv( - content.toString("utf8"), - options.scope, + const result = await this.brevoContactImportService.importContactsFromCsv({ + fileStream, + scope, redirectUrl, - targetGroups, - ); + targetGroupIds, + }); this.logger.log(result); } diff --git a/packages/api/src/brevo-contact/brevo-contact-import.controller.ts b/packages/api/src/brevo-contact/brevo-contact-import.controller.ts index da26417f..729894f5 100644 --- a/packages/api/src/brevo-contact/brevo-contact-import.controller.ts +++ b/packages/api/src/brevo-contact/brevo-contact-import.controller.ts @@ -1,14 +1,13 @@ import { CometValidationException, RequiredPermission } from "@comet/cms-api"; -import { EntityRepository } from "@mikro-orm/core"; -import { InjectRepository } from "@mikro-orm/nestjs"; import { Body, Controller, Inject, Post, Type, UploadedFile, UseInterceptors } from "@nestjs/common"; import { FileInterceptor } from "@nestjs/platform-express"; -import { TargetGroupInterface } from "src/target-group/entity/target-group-entity.factory"; +import { Readable } from "stream"; import { BrevoContactImportService } from "../brevo-contact/brevo-contact-import.service"; import { BrevoModuleConfig } from "../config/brevo-module.config"; import { BREVO_MODULE_CONFIG } from "../config/brevo-module.constants"; import { EmailCampaignScopeInterface } from "../types"; +import { CsvImportInformation } from "./brevo-contact-import.service"; export function createBrevoContactImportController({ Scope }: { Scope: Type }): Type { @Controller("brevo-contacts-csv") @@ -16,7 +15,6 @@ export function createBrevoContactImportController({ Scope }: { Scope: Type, ) {} @Post("upload") @@ -33,23 +31,25 @@ export function createBrevoContactImportController({ Scope }: { Scope: Type { - const content = file.buffer.toString("utf8"); + async upload( + @UploadedFile() file: Express.Multer.File, + @Body("scope") scope: string, + @Body("listIds") listIds?: string, + ): Promise { const parsedScope = JSON.parse(scope) as EmailCampaignScopeInterface; - const redirectUrl = this.config.brevo.resolveConfig(parsedScope).redirectUrlForImport; - const contacts = await this.brevoContactImportService.parseCsvToBrevoContacts(content, redirectUrl); - - if (contacts.length > 100) { - throw new CometValidationException("Too many contacts in file. Currently we only support 100 contacts at once."); - } - - let parsedListIds = undefined; - if (listIds) parsedListIds = JSON.parse(listIds) as string[]; - - const targetGroups = await this.targetGroupRepository.find({ id: { $in: parsedListIds } }); - await this.brevoContactImportService.importContactsFromCsv(content, parsedScope, redirectUrl, targetGroups); + let targetGroupIds = undefined; + if (listIds) targetGroupIds = JSON.parse(listIds) as string[]; + + const stream = Readable.from(file.buffer); + return this.brevoContactImportService.importContactsFromCsv({ + fileStream: stream, + scope: parsedScope, + redirectUrl, + targetGroupIds, + isAdminImport: true, + }); } } diff --git a/packages/api/src/brevo-contact/brevo-contact-import.service.ts b/packages/api/src/brevo-contact/brevo-contact-import.service.ts index c5cd963d..1a3fcf0b 100644 --- a/packages/api/src/brevo-contact/brevo-contact-import.service.ts +++ b/packages/api/src/brevo-contact/brevo-contact-import.service.ts @@ -1,23 +1,43 @@ -import { parseString } from "@fast-csv/parse"; +import * as csv from "@fast-csv/parse"; +import { EntityRepository } from "@mikro-orm/core"; +import { InjectRepository } from "@mikro-orm/nestjs"; import { Inject, Injectable } from "@nestjs/common"; -import { IsEmail, IsNotEmpty, validateOrReject } from "class-validator"; +import { IsEmail, IsNotEmpty, validateSync } from "class-validator"; +import isEqual from "lodash.isequal"; +import { TargetGroupInterface } from "src/target-group/entity/target-group-entity.factory"; +import { Readable } from "stream"; import { isErrorFromBrevo } from "../brevo-api/brevo-api.utils"; import { BrevoApiContactsService, CreateDoubleOptInContactData } from "../brevo-api/brevo-api-contact.service"; import { BrevoContactsService } from "../brevo-contact/brevo-contacts.service"; import { BrevoModuleConfig } from "../config/brevo-module.config"; import { BREVO_MODULE_CONFIG } from "../config/brevo-module.constants"; -import { TargetGroupInterface } from "../target-group/entity/target-group-entity.factory"; import { TargetGroupsService } from "../target-group/target-groups.service"; import { EmailCampaignScopeInterface } from "../types"; -class ValidateableRow { +class BasicValidateableRow { @IsEmail() @IsNotEmpty() email: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: any; + [key: string]: string; +} + +export interface CsvImportInformation { + created: number; + updated: number; + failed: number; + failedColumns: unknown[]; + errorMessage?: string; +} + +interface ImportContactsFromCsvParams { + fileStream: Readable; + scope: EmailCampaignScopeInterface; + redirectUrl: string; + targetGroupIds?: string[]; + isAdminImport?: boolean; } @Injectable() @@ -27,116 +47,165 @@ export class BrevoContactImportService { private readonly brevoApiContactsService: BrevoApiContactsService, private readonly brevoContactsService: BrevoContactsService, private readonly targetGroupsService: TargetGroupsService, + @InjectRepository("TargetGroup") private readonly targetGroupRepository: EntityRepository, ) {} - async importContactsFromCsv( - csvContent: string, - scope: EmailCampaignScopeInterface, - redirectUrl: string, - targetGroups: TargetGroupInterface[] = [], - ): Promise<{ created: number; updated: number; failed: number }> { - let created = 0; - let updated = 0; - let failed = 0; - const contacts = await this.parseCsvToBrevoContacts(csvContent, redirectUrl); - const targetGroupBrevoIds = await Promise.all( + async importContactsFromCsv({ + fileStream, + scope, + redirectUrl, + targetGroupIds = [], + isAdminImport = false, + }: ImportContactsFromCsvParams): Promise { + const failedColumns: unknown[] = []; + const targetGroups = await this.targetGroupRepository.find({ id: { $in: targetGroupIds } }); + + for (const targetGroup of targetGroups) { + if (targetGroup.isMainList) { + throw new Error("Main lists are not allowed as target groups for import"); + } + + if (!isEqual({ ...targetGroup.scope }, scope)) { + throw new Error("Target group scope does not match the scope of the import file"); + } + } + + const manuallyAssignedBrevoContacts = await Promise.all( targetGroups.map((targetGroup) => { return this.targetGroupsService.createIfNotExistsManuallyAssignedContactsTargetGroup(targetGroup); }), ); + const targetGroupBrevoIds = [...targetGroups.map((targetGroup) => targetGroup.brevoId), ...manuallyAssignedBrevoContacts]; + + const rows = fileStream.pipe(csv.parse({ headers: true, delimiter: ";", ignoreEmpty: true })).on("error", (error) => { + throw error; + }); - for (const contact of contacts) { + let created = 0; + let updated = 0; + let failed = 0; + for await (const row of rows) { + // This is a temporary solution. We should handle the import as a background job and allow importing more than 100 contacts + if (isAdminImport && created + updated + failed > 100) { + return { + created, + updated, + failed, + failedColumns, + errorMessage: + "Too many contacts. Currently we only support 100 contacts at once, the first 100 contacts were handled. Please split the file and try again with the remaining contacts.", + }; + } try { - let brevoContact; - try { - brevoContact = await this.brevoApiContactsService.findContact(contact.email, scope); - } catch (error) { - // Brevo throws 404 error if no contact was found - if (!isErrorFromBrevo(error)) { - throw error; - } - if (error.response.statusCode !== 404) { - throw error; - } - } - if (brevoContact && !brevoContact.emailBlacklisted) { - const mainTargetGroupForScope = await this.targetGroupsService.createIfNotExistMainTargetGroupForScope(scope); - - const updatedBrevoContact = await this.brevoApiContactsService.updateContact( - brevoContact.id, - { ...contact, listIds: [mainTargetGroupForScope.brevoId, ...targetGroupBrevoIds, ...brevoContact.listIds] }, - scope, - ); - if (updatedBrevoContact) updated++; - else failed++; - } else if (!brevoContact) { - const success = await this.brevoContactsService.createDoubleOptInContact({ - ...contact, - scope, - templateId: this.config.brevo.resolveConfig(scope).doubleOptInTemplateId, - listIds: targetGroupBrevoIds, - }); - if (success) created++; - else failed++; + const contactData = await this.processCsvRow(row, redirectUrl); + const result = await this.createOrUpdateBrevoContact(contactData, scope, targetGroupBrevoIds); + switch (result) { + case "created": + created++; + break; + case "updated": + updated++; + break; + case "error": + failedColumns.push(row); + failed++; + break; } - } catch (err) { - console.error(err); + } catch (validationError) { + console.error(validationError); + failedColumns.push(row); failed++; } } - return { created, updated, failed }; + if (created + updated + failed === 0) { + return { created, updated, failed, failedColumns, errorMessage: "No contacts found." }; + } + + return { created, updated, failed, failedColumns }; } - async parseCsvToBrevoContacts(csvContent: string, redirectUrl: string): Promise { - const brevoContacts: CreateDoubleOptInContactData[] = []; - - return new Promise((resolve, reject) => { - parseString(csvContent, { headers: true, delimiter: ";" }) - .on("error", (error) => { - console.error(error); - reject(error); - }) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .on("data", async (row: Record) => { - try { - const contactData = await this.processRow(row, redirectUrl); - brevoContacts.push(contactData); - } catch (validationError) { - console.error(validationError); - reject(validationError); - } - }) - .on("end", (rowCount: number) => { - console.log(`Parsed ${rowCount} rows`); - resolve(brevoContacts); + private async createOrUpdateBrevoContact( + contact: CreateDoubleOptInContactData, + scope: EmailCampaignScopeInterface, + targetGroupBrevoIds: number[], + ): Promise<"created" | "updated" | "error"> { + try { + let brevoContact; + try { + brevoContact = await this.brevoApiContactsService.findContact(contact.email, scope); + } catch (error) { + if (!isErrorFromBrevo(error)) { + throw error; + } + // Brevo throws 404 error if no contact was found + if (error.response.statusCode !== 404) { + throw error; + } + } + const mainTargetGroupForScope = await this.targetGroupsService.createIfNotExistMainTargetGroupForScope(scope); + if (brevoContact && !brevoContact.emailBlacklisted) { + const updatedBrevoContact = await this.brevoApiContactsService.updateContact( + brevoContact.id, + { ...contact, listIds: [mainTargetGroupForScope.brevoId, ...targetGroupBrevoIds, ...brevoContact.listIds] }, + scope, + ); + if (updatedBrevoContact) return "updated"; + } else if (!brevoContact) { + const success = await this.brevoContactsService.createDoubleOptInContact({ + ...contact, + scope, + templateId: this.config.brevo.resolveConfig(scope).doubleOptInTemplateId, + listIds: [mainTargetGroupForScope.brevoId, ...targetGroupBrevoIds], }); - }); + if (success) return "created"; + } + } catch (err) { + console.error(err); + } + return "error"; } - private async processRow( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - row: Record, - redirectUrlForImport: string, - ): Promise { - const { email, ...data } = await this.createAndValidateRow(row); + private async processCsvRow(row: Record, redirectUrlForImport: string): Promise { + const mappedRow = this.createValidateableCsvRowClass(); + + // Make all keys uppercase because all attributes have to be defined uppercase in brevo + // Make email lowercase, because that's how brevo expects it + for (const key in row) { + if (key.toLowerCase() === "email") { + mappedRow.email = row[key]; + } else { + mappedRow[key.toUpperCase()] = row[key]; + } + } + + const errors = validateSync(mappedRow); + + if (errors.length > 0) { + throw errors; + } + + const { email, ...data } = mappedRow; + return { - email, + email: mappedRow.email, redirectionUrl: redirectUrlForImport, attributes: { ...data }, }; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private async createAndValidateRow(row: Record): Promise { - const mappedRow = new ValidateableRow(); - Object.keys(row).forEach((key) => { - if (key.toLowerCase() === "email") { - mappedRow.email = row[key]; - } else { - mappedRow[key] = row[key]; + private createValidateableCsvRowClass(): BasicValidateableRow { + if (this.config.brevo.BrevoContactAttributes) { + const BrevoContactAttributesClass = this.config.brevo.BrevoContactAttributes; + class ValidateableRow extends BrevoContactAttributesClass { + @IsEmail() + @IsNotEmpty() + email: string; } - }); - await validateOrReject(mappedRow); - return mappedRow; + + return new ValidateableRow(); + } else { + class ValidateableRow extends BasicValidateableRow {} + return new ValidateableRow(); + } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dc1d22fa..f939c916 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1010,6 +1010,9 @@ importers: '@types/jest': specifier: ^29.5.0 version: 29.5.11 + '@types/lodash.isequal': + specifier: ^4.0.0 + version: 4.5.8 '@types/mime-db': specifier: ^1.43.5 version: 1.43.5 @@ -1049,6 +1052,9 @@ importers: jest-junit: specifier: ^15.0.0 version: 15.0.0 + lodash.isequal: + specifier: ^4.5.0 + version: 4.5.0 nestjs-console: specifier: ^8.0.0 version: 8.0.0(@nestjs/common@9.4.3)(@nestjs/core@9.4.3)