From f807d261382e0fdb8582e3c9ca2a9db555b4bb44 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Thu, 8 Oct 2026 14:53:58 +0200 Subject: [PATCH 1/6] Catalog import: row-level results, dry run and HTML rejection for categories, brands and collections --- .../ExportImport/BrandImportDataObject.cs | 138 ++++++- .../ExportImport/CategoryImportDataObject.cs | 142 +++++++- .../CollectionImportDataObject.cs | 138 ++++++- .../Services/ExportImport/ImportHtmlGuard.cs | 42 +++ .../Services/ExportImport/ImportRows.cs | 35 ++ .../Startup/StartupApplication.cs | 13 +- .../Interfaces/ExportImport/IRowImport.cs | 39 ++ .../BrandImportDataObjectTests.cs | 279 +++++++++++++- .../CategoryImportDataObjectTests.cs | 340 +++++++++++++++++- .../CollectionImportDataObjectTests.cs | 279 +++++++++++++- 10 files changed, 1410 insertions(+), 35 deletions(-) create mode 100644 src/Business/Grand.Business.Catalog/Services/ExportImport/ImportHtmlGuard.cs create mode 100644 src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs create mode 100644 src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs index 7a6cfddb99..a8f88eff8f 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs @@ -6,12 +6,14 @@ using Grand.Business.Core.Utilities.System; using Grand.Domain.Catalog; using Grand.Domain.Media; +using Grand.Domain.Seo; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; -public class BrandImportDataObject : IImportDataObject +public class BrandImportDataObject : IImportDataObject, IRowImport { private readonly IBrandLayoutService _brandLayoutService; private readonly IBrandService _brandService; @@ -19,6 +21,9 @@ public class BrandImportDataObject : IImportDataObject private readonly ISlugService _slugService; private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; + private readonly ImportHtmlGuard _htmlGuard; + private readonly SeoSettings _seoSettings; + private readonly ILogger _logger; public BrandImportDataObject( IBrandService brandService, @@ -26,7 +31,10 @@ public BrandImportDataObject( IBrandLayoutService brandLayoutService, ISlugService slugService, ISeNameService seNameService, - SecurityConfig securityConfig) + SecurityConfig securityConfig, + ImportHtmlGuard htmlGuard, + SeoSettings seoSettings, + ILogger logger) { _brandService = brandService; _pictureService = pictureService; @@ -34,27 +42,137 @@ public BrandImportDataObject( _slugService = slugService; _seNameService = seNameService; _securityConfig = securityConfig; + _htmlGuard = htmlGuard; + _seoSettings = seoSettings; + _logger = logger; } public async Task Execute(IEnumerable data) { - foreach (var item in data) await Import(item); + var result = await Import(data.ToList(), false, ImportMode.Panel); + ImportRows.LogRejected(_logger, "brand", result); } - private async Task Import(BrandDto brandDto) + /// Row API: an unknown Id is rejected, a row without Id is matched by SeName, else (best effort) by Name + public Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) { - var brand = await _brandService.GetBrandById(brandDto.Id); - var isNew = brand == null; + return Import(rows, dryRun, ImportMode.Row, cancellationToken); + } - if (brand == null) brand = brandDto.MapTo(); - else brandDto.MapTo(brand); + /// + /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. + /// + private enum ImportMode + { + Row, + Panel + } - if (!ValidBrand(brand)) return; + private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, BrandDto dto, bool dryRun, ImportMode mode) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + + var (existing, matchedByName) = await FindBrand(dto, mode); + if (matchedByName) + warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); + if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) + errors.Add($"Id '{dto.Id}' was not found."); + else if (existing == null && string.IsNullOrEmpty(dto.Name)) + errors.Add("Name is required for a new brand."); + if (dto.Name == "") + errors.Add("Name cannot be empty."); + + errors.AddRange(_htmlGuard.RichTextErrors((nameof(dto.Description), dto.Description), + (nameof(dto.BottomDescription), dto.BottomDescription))); + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName), + (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), + (nameof(dto.MetaTitle), dto.MetaTitle))); + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? dto.Id ?? "", key, errors, warnings); + + //an entity found by SeName keeps its own id + if (existing != null) dto.Id = existing.Id; + var brand = existing; + var isNew = brand == null; + if (brand == null) brand = dto.MapTo(); + else dto.MapTo(brand); + + if (!ValidBrand(brand)) + { + errors.Add("Name is required for a new brand."); + return new ImportRowResult(row, ImportRowStatus.Rejected, "", key, errors, warnings); + } if (isNew) await _brandService.InsertBrand(brand); else await _brandService.UpdateBrand(brand); - await UpdateBrandData(brandDto, brand); + await UpdateBrandData(dto, brand); + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, brand.Id, key, + errors, warnings); + } + + private static string Key(BrandDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.SeName) ? dto.SeName : dto.Id ?? ""; + } + + /// + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug + /// its Name would get - accepted only when the stored name is the same (any case) + /// + private async Task<(Brand brand, bool matchedByName)> FindBrand(BrandDto dto, ImportMode mode) + { + if (!string.IsNullOrEmpty(dto.Id)) + return (await _brandService.GetBrandById(dto.Id), false); + + if (mode == ImportMode.Panel) + return (null, false); + + if (!string.IsNullOrEmpty(dto.SeName)) + return (await FindBrandBySlug(dto.SeName), false); + + if (string.IsNullOrEmpty(dto.Name)) + return (null, false); + + var found = await FindBrandBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); + var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase); + return matches ? (found, true) : (null, false); + } + + private async Task FindBrandBySlug(string slug) + { + var url = await _slugService.GetBySlug(slug); + if (url is not { EntityName: EntityTypes.Brand } || string.IsNullOrEmpty(url.EntityId)) + return null; + + return await _brandService.GetBrandById(url.EntityId); } private async Task UpdateBrandData(BrandDto brandDto, Brand brand) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs index 13723cf594..e92402833b 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs @@ -6,12 +6,14 @@ using Grand.Business.Core.Utilities.System; using Grand.Domain.Catalog; using Grand.Domain.Media; +using Grand.Domain.Seo; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; -public class CategoryImportDataObject : IImportDataObject +public class CategoryImportDataObject : IImportDataObject, IRowImport { private readonly ICategoryLayoutService _categoryLayoutService; private readonly ICategoryService _categoryService; @@ -19,6 +21,9 @@ public class CategoryImportDataObject : IImportDataObject private readonly ISlugService _slugService; private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; + private readonly ImportHtmlGuard _htmlGuard; + private readonly SeoSettings _seoSettings; + private readonly ILogger _logger; public CategoryImportDataObject( ICategoryService categoryService, @@ -26,7 +31,10 @@ public CategoryImportDataObject( ICategoryLayoutService categoryLayoutService, ISlugService slugService, ISeNameService seNameService, - SecurityConfig securityConfig) + SecurityConfig securityConfig, + ImportHtmlGuard htmlGuard, + SeoSettings seoSettings, + ILogger logger) { _categoryService = categoryService; _pictureService = pictureService; @@ -34,26 +42,142 @@ public CategoryImportDataObject( _slugService = slugService; _seNameService = seNameService; _securityConfig = securityConfig; + _htmlGuard = htmlGuard; + _seoSettings = seoSettings; + _logger = logger; } public async Task Execute(IEnumerable data) { - foreach (var item in data) await Import(item); + var result = await Import(data.ToList(), false, ImportMode.Panel); + ImportRows.LogRejected(_logger, "category", result); } - private async Task Import(CategoryDto categoryDto) + /// Row API: an unknown Id is rejected, a row without Id is matched by SeName, else (best effort) by Name + public Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) { - var category = await _categoryService.GetCategoryById(categoryDto.Id); + return Import(rows, dryRun, ImportMode.Row, cancellationToken); + } + + /// + /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. + /// + private enum ImportMode + { + Row, + Panel + } + + private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, CategoryDto dto, bool dryRun, ImportMode mode) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + + var (existing, matchedByName) = await FindCategory(dto, mode); + if (matchedByName) + warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); + if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) + errors.Add($"Id '{dto.Id}' was not found."); + else if (existing == null && string.IsNullOrEmpty(dto.Name)) + errors.Add("Name is required for a new category."); + if (dto.Name == "") + errors.Add("Name cannot be empty."); + + errors.AddRange(_htmlGuard.RichTextErrors((nameof(dto.Description), dto.Description), + (nameof(dto.BottomDescription), dto.BottomDescription))); + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName), + (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), + (nameof(dto.MetaTitle), dto.MetaTitle))); + + if (!string.IsNullOrEmpty(dto.ParentCategoryId) && await _categoryService.GetCategoryById(dto.ParentCategoryId) == null) + warnings.Add($"ParentCategoryId '{dto.ParentCategoryId}' was not found and was cleared."); + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? dto.Id ?? "", key, errors, warnings); + + //an entity found by SeName keeps its own id + if (existing != null) dto.Id = existing.Id; + var category = existing; var isNew = category == null; - if (category == null) category = categoryDto.MapTo(); - else categoryDto.MapTo(category); + if (category == null) category = dto.MapTo(); + else dto.MapTo(category); - if (!ValidCategory(category)) return; + if (!ValidCategory(category)) + { + errors.Add("Name is required for a new category."); + return new ImportRowResult(row, ImportRowStatus.Rejected, "", key, errors, warnings); + } if (isNew) await _categoryService.InsertCategory(category); else await _categoryService.UpdateCategory(category); - await UpdateCategoryData(categoryDto, category); + await UpdateCategoryData(dto, category); + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, category.Id, key, + errors, warnings); + } + + private static string Key(CategoryDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.SeName) ? dto.SeName : dto.Id ?? ""; + } + + /// + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug + /// its Name would get - accepted only when the stored name is the same (any case) and, when the row names a + /// parent, under that parent + /// + private async Task<(Category category, bool matchedByName)> FindCategory(CategoryDto dto, ImportMode mode) + { + if (!string.IsNullOrEmpty(dto.Id)) + return (await _categoryService.GetCategoryById(dto.Id), false); + + if (mode == ImportMode.Panel) + return (null, false); + + if (!string.IsNullOrEmpty(dto.SeName)) + return (await FindCategoryBySlug(dto.SeName), false); + + if (string.IsNullOrEmpty(dto.Name)) + return (null, false); + + var found = await FindCategoryBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); + var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase) && + (string.IsNullOrEmpty(dto.ParentCategoryId) || dto.ParentCategoryId == found.ParentCategoryId); + return matches ? (found, true) : (null, false); + } + + private async Task FindCategoryBySlug(string slug) + { + var url = await _slugService.GetBySlug(slug); + if (url is not { EntityName: EntityTypes.Category } || string.IsNullOrEmpty(url.EntityId)) + return null; + + return await _categoryService.GetCategoryById(url.EntityId); } private async Task UpdateCategoryData(CategoryDto categoryDto, Category category) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs index ea49ac46c1..dea27b1403 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs @@ -6,12 +6,14 @@ using Grand.Business.Core.Utilities.System; using Grand.Domain.Catalog; using Grand.Domain.Media; +using Grand.Domain.Seo; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; -public class CollectionImportDataObject : IImportDataObject +public class CollectionImportDataObject : IImportDataObject, IRowImport { private readonly ICollectionLayoutService _collectionLayoutService; private readonly ICollectionService _collectionService; @@ -19,13 +21,19 @@ public class CollectionImportDataObject : IImportDataObject private readonly ISlugService _slugService; private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; + private readonly ImportHtmlGuard _htmlGuard; + private readonly SeoSettings _seoSettings; + private readonly ILogger _logger; public CollectionImportDataObject( ICollectionService collectionService, IPictureService pictureService, ICollectionLayoutService collectionLayoutService, ISlugService slugService, ISeNameService seNameService, - SecurityConfig securityConfig) + SecurityConfig securityConfig, + ImportHtmlGuard htmlGuard, + SeoSettings seoSettings, + ILogger logger) { _collectionService = collectionService; _pictureService = pictureService; @@ -34,27 +42,137 @@ public CollectionImportDataObject( _seNameService = seNameService; _securityConfig = securityConfig; + _htmlGuard = htmlGuard; + _seoSettings = seoSettings; + _logger = logger; } public async Task Execute(IEnumerable data) { - foreach (var item in data) await Import(item); + var result = await Import(data.ToList(), false, ImportMode.Panel); + ImportRows.LogRejected(_logger, "collection", result); } - private async Task Import(CollectionDto collectionDto) + /// Row API: an unknown Id is rejected, a row without Id is matched by SeName, else (best effort) by Name + public Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) { - var collection = await _collectionService.GetCollectionById(collectionDto.Id); - var isNew = collection == null; + return Import(rows, dryRun, ImportMode.Row, cancellationToken); + } - if (collection == null) collection = collectionDto.MapTo(); - else collectionDto.MapTo(collection); + /// + /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. + /// + private enum ImportMode + { + Row, + Panel + } - if (!ValidCollection(collection)) return; + private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, CollectionDto dto, bool dryRun, ImportMode mode) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + + var (existing, matchedByName) = await FindCollection(dto, mode); + if (matchedByName) + warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); + if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) + errors.Add($"Id '{dto.Id}' was not found."); + else if (existing == null && string.IsNullOrEmpty(dto.Name)) + errors.Add("Name is required for a new collection."); + if (dto.Name == "") + errors.Add("Name cannot be empty."); + + errors.AddRange(_htmlGuard.RichTextErrors((nameof(dto.Description), dto.Description), + (nameof(dto.BottomDescription), dto.BottomDescription))); + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName), + (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), + (nameof(dto.MetaTitle), dto.MetaTitle))); + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? dto.Id ?? "", key, errors, warnings); + + //an entity found by SeName keeps its own id + if (existing != null) dto.Id = existing.Id; + var collection = existing; + var isNew = collection == null; + if (collection == null) collection = dto.MapTo(); + else dto.MapTo(collection); + + if (!ValidCollection(collection)) + { + errors.Add("Name is required for a new collection."); + return new ImportRowResult(row, ImportRowStatus.Rejected, "", key, errors, warnings); + } if (isNew) await _collectionService.InsertCollection(collection); else await _collectionService.UpdateCollection(collection); - await UpdateCollectionData(collectionDto, collection); + await UpdateCollectionData(dto, collection); + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, collection.Id, key, + errors, warnings); + } + + private static string Key(CollectionDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.SeName) ? dto.SeName : dto.Id ?? ""; + } + + /// + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug + /// its Name would get - accepted only when the stored name is the same (any case) + /// + private async Task<(Collection collection, bool matchedByName)> FindCollection(CollectionDto dto, ImportMode mode) + { + if (!string.IsNullOrEmpty(dto.Id)) + return (await _collectionService.GetCollectionById(dto.Id), false); + + if (mode == ImportMode.Panel) + return (null, false); + + if (!string.IsNullOrEmpty(dto.SeName)) + return (await FindCollectionBySlug(dto.SeName), false); + + if (string.IsNullOrEmpty(dto.Name)) + return (null, false); + + var found = await FindCollectionBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); + var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase); + return matches ? (found, true) : (null, false); + } + + private async Task FindCollectionBySlug(string slug) + { + var url = await _slugService.GetBySlug(slug); + if (url is not { EntityName: EntityTypes.Collection } || string.IsNullOrEmpty(url.EntityId)) + return null; + + return await _collectionService.GetCollectionById(url.EntityId); } private async Task UpdateCollectionData(CollectionDto collectionDto, Collection collection) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportHtmlGuard.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportHtmlGuard.cs new file mode 100644 index 0000000000..ecc2a0c702 --- /dev/null +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportHtmlGuard.cs @@ -0,0 +1,42 @@ +using Grand.Infrastructure.Configuration; +using Grand.Infrastructure.Security; + +namespace Grand.Business.Catalog.Services.ExportImport; + +/// +/// Rejects imported values that carry markup outside the allowlist. Detection only: the row is rejected and the +/// field named, the value is never rewritten. Does nothing when is off. +/// +public class ImportHtmlGuard +{ + private readonly IHtmlSanitizationService _htmlSanitizationService; + private readonly SecurityConfig _securityConfig; + + public ImportHtmlGuard(IHtmlSanitizationService htmlSanitizationService, SecurityConfig securityConfig) + { + _htmlSanitizationService = htmlSanitizationService; + _securityConfig = securityConfig; + } + + /// Fields that may hold rich text (descriptions) + public IEnumerable RichTextErrors(params (string Field, string Value)[] fields) + { + return Errors(fields, _htmlSanitizationService.ContainsDisallowedRichText); + } + + /// Fields that must be plain text (names, SEO fields) + public IEnumerable PlainTextErrors(params (string Field, string Value)[] fields) + { + return Errors(fields, _htmlSanitizationService.ContainsMarkup); + } + + private IEnumerable Errors((string Field, string Value)[] fields, Func isDisallowed) + { + if (!_securityConfig.EnableHtmlSanitization) + yield break; + + foreach (var (field, value) in fields) + if (!string.IsNullOrEmpty(value) && isDisallowed(value)) + yield return $"{field} contains HTML that is not allowed."; + } +} diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs new file mode 100644 index 0000000000..be1ed56393 --- /dev/null +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs @@ -0,0 +1,35 @@ +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Seo; +using Grand.SharedKernel.Extensions; +using Microsoft.Extensions.Logging; + +namespace Grand.Business.Catalog.Services.ExportImport; + +/// Helpers the catalogue row imports share +internal static class ImportRows +{ + /// The slug a new category, brand or collection gets from its name (as ISeNameService.ValidateSeName builds it) + public static string NameSlug(string name, SeoSettings seoSettings) + { + var slug = SeoExtensions.GenerateSlug(name, seoSettings.ConvertNonWesternChars, + seoSettings.AllowUnicodeCharsInUrls, seoSettings.AllowSlashChar, seoSettings.SeoCharConversion); + return CommonHelper.EnsureMaximumLength(slug, 200); + } + + public static string MatchedByNameWarning(string name) + { + return $"Matched existing '{name}' by name."; + } + + /// The panel's spreadsheet import has no per-row result to show; its rejected rows go to the log + public static void LogRejected(ILogger logger, string entity, ImportBatchResult result) + { + var rejected = result.Rows.Where(r => r.Status == ImportRowStatus.Rejected).ToList(); + if (rejected.Count == 0) + return; + + logger.LogWarning("The {Entity} spreadsheet import skipped {Count} row(s): {Rows}", entity, rejected.Count, + string.Join("; ", rejected.Select(r => $"row {r.Row} ({r.Key}): {string.Join(" ", r.Errors)}"))); + } +} diff --git a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs index db8fcbb4ba..10eb3134db 100644 --- a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs @@ -99,9 +99,16 @@ private static void RegisterExportImport(IServiceCollection serviceCollection) serviceCollection.AddScoped, CollectionSchemaProperty>(); serviceCollection.AddScoped, ProductSchemaProperty>(); - serviceCollection.AddScoped, CategoryImportDataObject>(); - serviceCollection.AddScoped, BrandImportDataObject>(); - serviceCollection.AddScoped, CollectionImportDataObject>(); + serviceCollection.AddScoped(); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped(); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped(); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped(); serviceCollection.AddScoped, ProductImportDataObject>(); } } \ No newline at end of file diff --git a/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs b/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs new file mode 100644 index 0000000000..8b48e26bad --- /dev/null +++ b/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs @@ -0,0 +1,39 @@ +namespace Grand.Business.Core.Interfaces.ExportImport; + +public enum ImportRowStatus +{ + Created, + Updated, + Rejected +} + +/// 1-based position of the row in the submitted batch +/// Identifier of the created or updated entity; empty for a rejected new row, or a dry-run new row without an Id +/// Human-readable key of the row (name, else SeName, else id) +public sealed record ImportRowResult(int Row, ImportRowStatus Status, string Id, string Key, + IReadOnlyList Errors, IReadOnlyList Warnings) +{ + public const string NotProcessedError = "Not processed: the batch time limit was reached; send these rows again."; + + /// A row the import did not reach because its cancellation token fired; nothing was written for it + public static ImportRowResult NotProcessed(int row, string id, string key) + { + return new ImportRowResult(row, ImportRowStatus.Rejected, id ?? "", key, [NotProcessedError], []); + } +} + +public sealed record ImportBatchResult(bool DryRun, IReadOnlyList Rows) +{ + public int Created => Rows.Count(r => r.Status == ImportRowStatus.Created); + public int Updated => Rows.Count(r => r.Status == ImportRowStatus.Updated); + public int Rejected => Rows.Count(r => r.Status == ImportRowStatus.Rejected); +} + +/// +/// Row-level import with validation and dry run; the panel's Execute runs it with dryRun = false. A cancelled +/// token does not throw: the rows not reached yet come back rejected with +/// +public interface IRowImport +{ + Task Import(IReadOnlyList rows, bool dryRun, CancellationToken cancellationToken = default); +} diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs index e826064055..cab6445815 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs @@ -6,6 +6,7 @@ using Grand.Business.Core.Interfaces.Catalog.Brands; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Seo; +using Grand.Business.Core.Interfaces.ExportImport; using Grand.Business.Core.Interfaces.Storage; using Grand.Data; using Grand.Data.Tests.MongoDb; @@ -18,9 +19,11 @@ using Grand.Infrastructure.Caching; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Grand.Infrastructure.Security; using Grand.Infrastructure.Tests.Caching; using Grand.Infrastructure.TypeSearch; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -42,10 +45,13 @@ public class BrandImportDataObjectTests private Mock _slugServiceMock; private Mock _workContextMock; private ISeNameService _seNameService; + private SecurityConfig _securityConfig; + private Mock> _loggerMock; [TestInitialize] public void Init() { InitAutoMapper(); + _securityConfig = new SecurityConfig(); _repository = new MongoDBRepositoryTest(); @@ -65,7 +71,8 @@ public void Init() _seNameService = new SeNameService(_slugServiceMock.Object, _languageServiceMock.Object, new SeoSettings()); _brandImportDataObject = new BrandImportDataObject(_brandService, _pictureServiceMock.Object, _brandLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, - new SecurityConfig()); + _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), + new SeoSettings(), (_loggerMock = new Mock>()).Object); } [TestMethod] @@ -167,6 +174,276 @@ public async Task ExecuteTest_Import_Insert_Update() Assert.IsFalse(_repository.Table.FirstOrDefault(x => x.Id == brand3.Id).Published); } + private void SetupLayouts() + { + _brandLayoutServiceMock.Setup(c => c.GetBrandLayoutById(It.IsAny())) + .Returns(Task.FromResult(new BrandLayout())); + _brandLayoutServiceMock.Setup(c => c.GetAllBrandLayouts()) + .Returns(Task.FromResult>(new List { new() })); + _languageServiceMock.Setup(c => c.GetAllLanguages(It.IsAny(), It.IsAny())) + .Returns(Task.FromResult>(new List())); + } + + [TestMethod] + public async Task Import_NewRow_IsCreatedWithId() + { + SetupLayouts(); + + var result = await _brandImportDataObject.Import(new List { new() { Name = "new one" } }, false); + + Assert.AreEqual(1, result.Created); + var row = result.Rows[0]; + Assert.AreEqual(1, row.Row); + Assert.AreEqual(ImportRowStatus.Created, row.Status); + Assert.AreEqual("new one", row.Key); + Assert.IsFalse(string.IsNullOrEmpty(row.Id)); + Assert.AreEqual(row.Id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Import_ExistingById_IsUpdated() + { + SetupLayouts(); + var existing = new Brand { Name = "old" }; + await _brandService.InsertBrand(existing); + + var result = await _brandImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_ExistingBySeName_IsUpdatedWithSameId() + { + SetupLayouts(); + var existing = new Brand { Name = "old", SeName = "my-slug" }; + await _brandService.InsertBrand(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Brand", Slug = "my-slug" })); + + var result = await _brandImportDataObject.Import( + new List { new() { SeName = "my-slug", Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_SeNameOfAnotherEntityType_IsNotMatched() + { + SetupLayouts(); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = "x", EntityName = "Product", Slug = "my-slug" })); + + var result = await _brandImportDataObject.Import( + new List { new() { SeName = "my-slug" } }, true); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_NewRowWithoutName_IsRejected() + { + SetupLayouts(); + + var result = await _brandImportDataObject.Import(new List { new() { Published = true } }, false); + + Assert.AreEqual(1, result.Rejected); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_ScriptInDescription_RejectsRowNamingFieldAndContinues() + { + SetupLayouts(); + var rows = new List { + new() { Name = "bad", Description = "

hi

" }, + new() { Name = "good", Description = "

fine

" } + }; + + var result = await _brandImportDataObject.Import(rows, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Description"))); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); + Assert.AreEqual(2, result.Rows[1].Row); + Assert.AreEqual("good", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_MarkupInName_IsRejected() + { + SetupLayouts(); + + var result = await _brandImportDataObject.Import( + new List { new() { Name = "bold" } }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_SanitizationDisabled_AcceptsMarkup() + { + SetupLayouts(); + _securityConfig.EnableHtmlSanitization = false; + + var result = await _brandImportDataObject.Import( + new List { new() { Name = "x", Description = "" } }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_DryRun_ReportsStatusesWithoutWriting() + { + SetupLayouts(); + var existing = new Brand { Name = "old" }; + await _brandService.InsertBrand(existing); + var rows = new List { + new() { Name = "brand new", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Id = existing.Id, Name = "renamed", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Published = true } + }; + + var result = await _brandImportDataObject.Import(rows, true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[1].Status); + Assert.AreEqual(existing.Id, result.Rows[1].Id); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single().Name); + _slugServiceMock.Verify(c => c.SaveSlug(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _pictureServiceMock.VerifyNoOtherCalls(); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_UnknownId_IsRejected(bool dryRun) + { + SetupLayouts(); + + var result = await _brandImportDataObject.Import( + new List { new() { Id = "nope", Name = "x" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Id 'nope' was not found.", result.Rows[0].Errors); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Execute_UnknownId_CreatesEntityWithThatId() + { + SetupLayouts(); + var id = MongoDB.Bson.ObjectId.GenerateNewId().ToString(); + + await _brandImportDataObject.Execute(new List { new() { Id = id, Name = "x" } }); + + Assert.AreEqual(id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Execute_NoIdButExistingSeName_CreatesNewEntity() + { + SetupLayouts(); + var existing = new Brand { Name = "old", SeName = "my-slug" }; + await _brandService.InsertBrand(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Brand", Slug = "my-slug" })); + + await _brandImportDataObject.Execute(new List { new() { SeName = "my-slug", Name = "other" } }); + + Assert.AreEqual(2, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single(x => x.Id == existing.Id).Name); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_EmptyNameOnExisting_IsRejected(bool dryRun) + { + SetupLayouts(); + var existing = new Brand { Name = "old" }; + await _brandService.InsertBrand(existing); + + var result = await _brandImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Name cannot be empty.", result.Rows[0].Errors); + Assert.AreEqual("old", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + SetupLayouts(); + using var cts = new CancellationTokenSource(); + //the layout lookup runs while row 1 is saved + _brandLayoutServiceMock.Setup(c => c.GetAllBrandLayouts()) + .Callback(() => cts.Cancel()) + .Returns(Task.FromResult>(new List { new() })); + + var result = await _brandImportDataObject.Import( + new List { new() { Name = "one" }, new() { Name = "two" }, new() { Name = "three" } }, false, + cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual("three", result.Rows[2].Key); + Assert.AreEqual("Not processed: the batch time limit was reached; send these rows again.", + result.Rows[2].Errors.Single()); + Assert.AreEqual("one", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Execute_RejectedRows_LogsOneWarningWithRowNumbersAndReasons() + { + SetupLayouts(); + + await _brandImportDataObject.Execute(new List { + new() { Name = "good" }, + new() { Name = "bad", Description = "" } + }); + + Assert.AreEqual("good", _repository.Table.Single().Name); + _loggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains("row 2") && v.ToString()!.Contains("Description")), + It.IsAny(), It.IsAny>()), Times.Once); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_NameOnlyResent_MatchesExistingByName(bool dryRun) + { + SetupLayouts(); + var existing = new Brand { Name = "Acme Co", SeName = "acme-co" }; + await _brandService.InsertBrand(existing); + _slugServiceMock.Setup(c => c.GetBySlug("acme-co")) + .ReturnsAsync(new EntityUrl { EntityId = existing.Id, EntityName = "Brand", Slug = "acme-co" }); + + var result = await _brandImportDataObject.Import( + new List { new() { Name = "ACME CO", DisplayOrder = 7 } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.Contains("Matched existing 'Acme Co' by name.", result.Rows[0].Warnings); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual(dryRun ? 0 : 7, _repository.Table.Single().DisplayOrder); + } + private void InitAutoMapper() { var typeSearcher = new TypeSearcher(); diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs index 044b141843..d95be9e0fc 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs @@ -7,6 +7,7 @@ using Grand.Business.Core.Interfaces.Catalog.Categories; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Seo; +using Grand.Business.Core.Interfaces.ExportImport; using Grand.Business.Core.Interfaces.Storage; using Grand.Data; using Grand.Data.Tests.MongoDb; @@ -19,9 +20,11 @@ using Grand.Infrastructure.Caching; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Grand.Infrastructure.Security; using Grand.Infrastructure.Tests.Caching; using Grand.Infrastructure.TypeSearch; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -43,10 +46,13 @@ public class CategoryImportDataObjectTests private Mock _slugServiceMock; private Mock _workContextMock; private ISeNameService _seNameService; + private SecurityConfig _securityConfig; + private Mock> _loggerMock; [TestInitialize] public void Init() { InitAutoMapper(); + _securityConfig = new SecurityConfig(); _repository = new MongoDBRepositoryTest(); @@ -66,7 +72,8 @@ public void Init() _seNameService = new SeNameService(_slugServiceMock.Object, _languageServiceMock.Object, new SeoSettings()); _categoryImportDataObject = new CategoryImportDataObject(_categoryService, _pictureServiceMock.Object, _categoryLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, - new SecurityConfig()); + _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), + new SeoSettings(), (_loggerMock = new Mock>()).Object); } [TestMethod] @@ -168,6 +175,337 @@ public async Task ExecuteTest_Import_Insert_Update() Assert.IsFalse(_repository.Table.FirstOrDefault(x => x.Id == category3.Id).Published); } + private void SetupLayouts() + { + _categoryLayoutServiceMock.Setup(c => c.GetCategoryLayoutById(It.IsAny())) + .Returns(Task.FromResult(new CategoryLayout())); + _categoryLayoutServiceMock.Setup(c => c.GetAllCategoryLayouts()) + .Returns(Task.FromResult>(new List { new() })); + _languageServiceMock.Setup(c => c.GetAllLanguages(It.IsAny(), It.IsAny())) + .Returns(Task.FromResult>(new List())); + } + + [TestMethod] + public async Task Import_NewRow_IsCreatedWithId() + { + SetupLayouts(); + + var result = await _categoryImportDataObject.Import(new List { new() { Name = "new one" } }, false); + + Assert.AreEqual(1, result.Created); + var row = result.Rows[0]; + Assert.AreEqual(1, row.Row); + Assert.AreEqual(ImportRowStatus.Created, row.Status); + Assert.AreEqual("new one", row.Key); + Assert.IsFalse(string.IsNullOrEmpty(row.Id)); + Assert.AreEqual(row.Id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Import_ExistingById_IsUpdated() + { + SetupLayouts(); + var existing = new Category { Name = "old" }; + await _categoryService.InsertCategory(existing); + + var result = await _categoryImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_ExistingBySeName_IsUpdatedWithSameId() + { + SetupLayouts(); + var existing = new Category { Name = "old", SeName = "my-slug" }; + await _categoryService.InsertCategory(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Category", Slug = "my-slug" })); + + var result = await _categoryImportDataObject.Import( + new List { new() { SeName = "my-slug", Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_SeNameOfAnotherEntityType_IsNotMatched() + { + SetupLayouts(); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = "x", EntityName = "Product", Slug = "my-slug" })); + + var result = await _categoryImportDataObject.Import( + new List { new() { SeName = "my-slug" } }, true); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_NewRowWithoutName_IsRejected() + { + SetupLayouts(); + + var result = await _categoryImportDataObject.Import(new List { new() { Published = true } }, false); + + Assert.AreEqual(1, result.Rejected); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_ScriptInDescription_RejectsRowNamingFieldAndContinues() + { + SetupLayouts(); + var rows = new List { + new() { Name = "bad", Description = "

hi

" }, + new() { Name = "good", Description = "

fine

" } + }; + + var result = await _categoryImportDataObject.Import(rows, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Description"))); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); + Assert.AreEqual(2, result.Rows[1].Row); + Assert.AreEqual("good", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_MarkupInName_IsRejected() + { + SetupLayouts(); + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "bold" } }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_SanitizationDisabled_AcceptsMarkup() + { + SetupLayouts(); + _securityConfig.EnableHtmlSanitization = false; + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "x", Description = "" } }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_DryRun_ReportsStatusesWithoutWriting() + { + SetupLayouts(); + var existing = new Category { Name = "old" }; + await _categoryService.InsertCategory(existing); + var rows = new List { + new() { Name = "brand new", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Id = existing.Id, Name = "renamed", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Published = true } + }; + + var result = await _categoryImportDataObject.Import(rows, true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[1].Status); + Assert.AreEqual(existing.Id, result.Rows[1].Id); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single().Name); + _slugServiceMock.Verify(c => c.SaveSlug(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _pictureServiceMock.VerifyNoOtherCalls(); + } + + [TestMethod] + public async Task Import_UnknownParentCategory_WarnsAndClearsIt() + { + SetupLayouts(); + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "child", ParentCategoryId = "missing" } }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.HasCount(1, result.Rows[0].Warnings); + Assert.IsTrue(string.IsNullOrEmpty(_repository.Table.Single().ParentCategoryId)); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_UnknownId_IsRejected(bool dryRun) + { + SetupLayouts(); + + var result = await _categoryImportDataObject.Import( + new List { new() { Id = "nope", Name = "x" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Id 'nope' was not found.", result.Rows[0].Errors); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Execute_UnknownId_CreatesEntityWithThatId() + { + SetupLayouts(); + var id = MongoDB.Bson.ObjectId.GenerateNewId().ToString(); + + await _categoryImportDataObject.Execute(new List { new() { Id = id, Name = "x" } }); + + Assert.AreEqual(id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Execute_NoIdButExistingSeName_CreatesNewEntity() + { + SetupLayouts(); + var existing = new Category { Name = "old", SeName = "my-slug" }; + await _categoryService.InsertCategory(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Category", Slug = "my-slug" })); + + await _categoryImportDataObject.Execute(new List { new() { SeName = "my-slug", Name = "other" } }); + + Assert.AreEqual(2, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single(x => x.Id == existing.Id).Name); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_EmptyNameOnExisting_IsRejected(bool dryRun) + { + SetupLayouts(); + var existing = new Category { Name = "old" }; + await _categoryService.InsertCategory(existing); + + var result = await _categoryImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Name cannot be empty.", result.Rows[0].Errors); + Assert.AreEqual("old", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + SetupLayouts(); + using var cts = new CancellationTokenSource(); + //the layout lookup runs while row 1 is saved + _categoryLayoutServiceMock.Setup(c => c.GetAllCategoryLayouts()) + .Callback(() => cts.Cancel()) + .Returns(Task.FromResult>(new List { new() })); + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "one" }, new() { Name = "two" }, new() { Name = "three" } }, false, + cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual(3, result.Rows[2].Row); + Assert.AreEqual("three", result.Rows[2].Key); + Assert.AreEqual("Not processed: the batch time limit was reached; send these rows again.", + result.Rows[1].Errors.Single()); + Assert.AreEqual("one", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Execute_RejectedRows_LogsOneWarningWithRowNumbersAndReasons() + { + SetupLayouts(); + + await _categoryImportDataObject.Execute(new List { + new() { Name = "good" }, + new() { Name = "bad", Description = "" } + }); + + Assert.AreEqual("good", _repository.Table.Single().Name); + _loggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains("row 2") && v.ToString()!.Contains("Description")), + It.IsAny(), It.IsAny>()), Times.Once); + } + + [TestMethod] + public async Task Execute_NoRejectedRows_LogsNoWarning() + { + SetupLayouts(); + + await _categoryImportDataObject.Execute(new List { new() { Name = "good" } }); + + _loggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny>()), Times.Never); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_NameOnlyResent_MatchesExistingByName(bool dryRun) + { + SetupLayouts(); + var existing = new Category { Name = "Running Shoes", SeName = "running-shoes" }; + await _categoryService.InsertCategory(existing); + _slugServiceMock.Setup(c => c.GetBySlug("running-shoes")) + .ReturnsAsync(new EntityUrl { EntityId = existing.Id, EntityName = "Category", Slug = "running-shoes" }); + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "running shoes", DisplayOrder = 7 } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.Contains("Matched existing 'Running Shoes' by name.", result.Rows[0].Warnings); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual(dryRun ? 0 : 7, _repository.Table.Single().DisplayOrder); + } + + [TestMethod] + public async Task Import_SameNameUnderAnotherParent_IsCreated() + { + SetupLayouts(); + var men = new Category { Name = "Men" }; + await _categoryService.InsertCategory(men); + var women = new Category { Name = "Women" }; + await _categoryService.InsertCategory(women); + var existing = new Category { Name = "Shoes", SeName = "shoes", ParentCategoryId = men.Id }; + await _categoryService.InsertCategory(existing); + _slugServiceMock.Setup(c => c.GetBySlug("shoes")) + .ReturnsAsync(new EntityUrl { EntityId = existing.Id, EntityName = "Category", Slug = "shoes" }); + + var result = await _categoryImportDataObject.Import( + new List { new() { Name = "Shoes", ParentCategoryId = women.Id } }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreNotEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual(2, _repository.Table.Count(c => c.Name == "Shoes")); + Assert.IsFalse(result.Rows[0].Warnings.Any(w => w.StartsWith("Matched existing"))); + } + + [TestMethod] + public async Task Import_SlugOfDifferentlyNamedCategory_IsNotMatchedByName() + { + SetupLayouts(); + var existing = new Category { Name = "Shoes!", SeName = "shoes" }; + await _categoryService.InsertCategory(existing); + _slugServiceMock.Setup(c => c.GetBySlug("shoes")) + .ReturnsAsync(new EntityUrl { EntityId = existing.Id, EntityName = "Category", Slug = "shoes" }); + + var result = await _categoryImportDataObject.Import(new List { new() { Name = "Shoes" } }, true); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + } + private void InitAutoMapper() { var typeSearcher = new TypeSearcher(); diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs index d5e069770d..ceb0f8bf76 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs @@ -7,6 +7,7 @@ using Grand.Business.Core.Interfaces.Catalog.Collections; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Seo; +using Grand.Business.Core.Interfaces.ExportImport; using Grand.Business.Core.Interfaces.Storage; using Grand.Data; using Grand.Data.Tests.MongoDb; @@ -19,9 +20,11 @@ using Grand.Infrastructure.Caching; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Grand.Infrastructure.Security; using Grand.Infrastructure.Tests.Caching; using Grand.Infrastructure.TypeSearch; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -43,10 +46,13 @@ public class CollectionImportDataObjectTests private Mock _slugServiceMock; private Mock _workContextMock; private ISeNameService _seNameService; + private SecurityConfig _securityConfig; + private Mock> _loggerMock; [TestInitialize] public void Init() { InitAutoMapper(); + _securityConfig = new SecurityConfig(); _repository = new MongoDBRepositoryTest(); @@ -66,7 +72,8 @@ public void Init() _seNameService = new SeNameService(_slugServiceMock.Object, _languageServiceMock.Object, new SeoSettings()); _collectionImportDataObject = new CollectionImportDataObject(_collectionService, _pictureServiceMock.Object, _collectionLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, - new SecurityConfig()); + _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), + new SeoSettings(), (_loggerMock = new Mock>()).Object); } [TestMethod] @@ -172,6 +179,276 @@ public async Task ExecuteTest_Import_Insert_Update() Assert.IsFalse(_repository.Table.FirstOrDefault(x => x.Id == collection3.Id).Published); } + private void SetupLayouts() + { + _collectionLayoutServiceMock.Setup(c => c.GetCollectionLayoutById(It.IsAny())) + .Returns(Task.FromResult(new CollectionLayout())); + _collectionLayoutServiceMock.Setup(c => c.GetAllCollectionLayouts()) + .Returns(Task.FromResult>(new List { new() })); + _languageServiceMock.Setup(c => c.GetAllLanguages(It.IsAny(), It.IsAny())) + .Returns(Task.FromResult>(new List())); + } + + [TestMethod] + public async Task Import_NewRow_IsCreatedWithId() + { + SetupLayouts(); + + var result = await _collectionImportDataObject.Import(new List { new() { Name = "new one" } }, false); + + Assert.AreEqual(1, result.Created); + var row = result.Rows[0]; + Assert.AreEqual(1, row.Row); + Assert.AreEqual(ImportRowStatus.Created, row.Status); + Assert.AreEqual("new one", row.Key); + Assert.IsFalse(string.IsNullOrEmpty(row.Id)); + Assert.AreEqual(row.Id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Import_ExistingById_IsUpdated() + { + SetupLayouts(); + var existing = new Collection { Name = "old" }; + await _collectionService.InsertCollection(existing); + + var result = await _collectionImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_ExistingBySeName_IsUpdatedWithSameId() + { + SetupLayouts(); + var existing = new Collection { Name = "old", SeName = "my-slug" }; + await _collectionService.InsertCollection(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Collection", Slug = "my-slug" })); + + var result = await _collectionImportDataObject.Import( + new List { new() { SeName = "my-slug", Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_SeNameOfAnotherEntityType_IsNotMatched() + { + SetupLayouts(); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = "x", EntityName = "Product", Slug = "my-slug" })); + + var result = await _collectionImportDataObject.Import( + new List { new() { SeName = "my-slug" } }, true); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_NewRowWithoutName_IsRejected() + { + SetupLayouts(); + + var result = await _collectionImportDataObject.Import(new List { new() { Published = true } }, false); + + Assert.AreEqual(1, result.Rejected); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_ScriptInDescription_RejectsRowNamingFieldAndContinues() + { + SetupLayouts(); + var rows = new List { + new() { Name = "bad", Description = "

hi

" }, + new() { Name = "good", Description = "

fine

" } + }; + + var result = await _collectionImportDataObject.Import(rows, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Description"))); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); + Assert.AreEqual(2, result.Rows[1].Row); + Assert.AreEqual("good", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_MarkupInName_IsRejected() + { + SetupLayouts(); + + var result = await _collectionImportDataObject.Import( + new List { new() { Name = "bold" } }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Import_SanitizationDisabled_AcceptsMarkup() + { + SetupLayouts(); + _securityConfig.EnableHtmlSanitization = false; + + var result = await _collectionImportDataObject.Import( + new List { new() { Name = "x", Description = "" } }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_DryRun_ReportsStatusesWithoutWriting() + { + SetupLayouts(); + var existing = new Collection { Name = "old" }; + await _collectionService.InsertCollection(existing); + var rows = new List { + new() { Name = "brand new", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Id = existing.Id, Name = "renamed", Picture = "http://127.0.0.1:1/pic.png" }, + new() { Published = true } + }; + + var result = await _collectionImportDataObject.Import(rows, true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[1].Status); + Assert.AreEqual(existing.Id, result.Rows[1].Id); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single().Name); + _slugServiceMock.Verify(c => c.SaveSlug(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _pictureServiceMock.VerifyNoOtherCalls(); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_UnknownId_IsRejected(bool dryRun) + { + SetupLayouts(); + + var result = await _collectionImportDataObject.Import( + new List { new() { Id = "nope", Name = "x" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Id 'nope' was not found.", result.Rows[0].Errors); + Assert.IsEmpty(_repository.Table); + } + + [TestMethod] + public async Task Execute_UnknownId_CreatesEntityWithThatId() + { + SetupLayouts(); + var id = MongoDB.Bson.ObjectId.GenerateNewId().ToString(); + + await _collectionImportDataObject.Execute(new List { new() { Id = id, Name = "x" } }); + + Assert.AreEqual(id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Execute_NoIdButExistingSeName_CreatesNewEntity() + { + SetupLayouts(); + var existing = new Collection { Name = "old", SeName = "my-slug" }; + await _collectionService.InsertCollection(existing); + _slugServiceMock.Setup(c => c.GetBySlug("my-slug")) + .Returns(Task.FromResult(new EntityUrl { EntityId = existing.Id, EntityName = "Collection", Slug = "my-slug" })); + + await _collectionImportDataObject.Execute(new List { new() { SeName = "my-slug", Name = "other" } }); + + Assert.AreEqual(2, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single(x => x.Id == existing.Id).Name); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_EmptyNameOnExisting_IsRejected(bool dryRun) + { + SetupLayouts(); + var existing = new Collection { Name = "old" }; + await _collectionService.InsertCollection(existing); + + var result = await _collectionImportDataObject.Import( + new List { new() { Id = existing.Id, Name = "" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Name cannot be empty.", result.Rows[0].Errors); + Assert.AreEqual("old", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + SetupLayouts(); + using var cts = new CancellationTokenSource(); + //the layout lookup runs while row 1 is saved + _collectionLayoutServiceMock.Setup(c => c.GetAllCollectionLayouts()) + .Callback(() => cts.Cancel()) + .Returns(Task.FromResult>(new List { new() })); + + var result = await _collectionImportDataObject.Import( + new List { new() { Name = "one" }, new() { Name = "two" }, new() { Name = "three" } }, false, + cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual("three", result.Rows[2].Key); + Assert.AreEqual("Not processed: the batch time limit was reached; send these rows again.", + result.Rows[2].Errors.Single()); + Assert.AreEqual("one", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Execute_RejectedRows_LogsOneWarningWithRowNumbersAndReasons() + { + SetupLayouts(); + + await _collectionImportDataObject.Execute(new List { + new() { Name = "good" }, + new() { Name = "bad", Description = "" } + }); + + Assert.AreEqual("good", _repository.Table.Single().Name); + _loggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains("row 2") && v.ToString()!.Contains("Description")), + It.IsAny(), It.IsAny>()), Times.Once); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_NameOnlyResent_MatchesExistingByName(bool dryRun) + { + SetupLayouts(); + var existing = new Collection { Name = "Acme Co", SeName = "acme-co" }; + await _collectionService.InsertCollection(existing); + _slugServiceMock.Setup(c => c.GetBySlug("acme-co")) + .ReturnsAsync(new EntityUrl { EntityId = existing.Id, EntityName = "Collection", Slug = "acme-co" }); + + var result = await _collectionImportDataObject.Import( + new List { new() { Name = "ACME CO", DisplayOrder = 7 } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.Contains("Matched existing 'Acme Co' by name.", result.Rows[0].Warnings); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual(dryRun ? 0 : 7, _repository.Table.Single().DisplayOrder); + } + private void InitAutoMapper() { var typeSearcher = new TypeSearcher(); From 0358b70dd67ae6f3c9958e1464c0cbb41a5fa0fe Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Thu, 8 Oct 2026 14:55:10 +0200 Subject: [PATCH 2/6] Catalog import: product rows - unknown ids rejected, SeName/Sku matching, create defaults of the panel, picture urls --- .../ExportImport/ProductImportDataObject.cs | 210 +++++++- .../Startup/StartupApplication.cs | 4 +- .../Grand.Business.Core/Dto/ProductDto.cs | 3 + .../ProductImportDataObjectTests.cs | 502 +++++++++++++++++- 4 files changed, 696 insertions(+), 23 deletions(-) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs index fb31d7b827..72541ee838 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs @@ -12,13 +12,18 @@ using Grand.Business.Core.Utilities.System; using Grand.Domain.Catalog; using Grand.Domain.Common; +using Grand.Domain.Tax; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; -public class ProductImportDataObject : IImportDataObject +public class ProductImportDataObject : IImportDataObject, IRowImport { + /// Most picture downloads one Import call performs; later pictures get a warning instead + public const int MaxPictureDownloadsPerBatch = 20; + private readonly IBrandService _brandService; private readonly ICategoryService _categoryService; private readonly ICollectionService _collectionService; @@ -34,6 +39,12 @@ public class ProductImportDataObject : IImportDataObject private readonly IWarehouseService _warehouseService; private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; + private readonly TaxSettings _taxSettings; + private readonly ImportHtmlGuard _htmlGuard; + private readonly ILogger _logger; + private readonly Func> _downloadImage; + + /// Test seam; the default downloads through public ProductImportDataObject( IProductService productService, IPictureService pictureService, @@ -49,7 +60,11 @@ public ProductImportDataObject( ICollectionService collectionService, IProductCollectionService productCollectionService, ISeNameService seNameService, - SecurityConfig securityConfig) + SecurityConfig securityConfig, + TaxSettings taxSettings, + ImportHtmlGuard htmlGuard, + ILogger logger, + Func> downloadImage = null) { _productService = productService; _pictureService = pictureService; @@ -66,32 +81,175 @@ public ProductImportDataObject( _productCollectionService = productCollectionService; _seNameService = seNameService; _securityConfig = securityConfig; + _taxSettings = taxSettings; + _htmlGuard = htmlGuard; + _logger = logger; + //only http(s) URLs to public or explicitly allowed hosts are downloaded - a local path would let the + //import read any file on the server + _downloadImage = downloadImage ?? + (url => DownloadUrl.DownloadImage(url, _securityConfig.PictureImportAllowedPrivateHosts)); } public async Task Execute(IEnumerable data) { - foreach (var item in data) await Import(item); + var result = await Import(data.ToList(), false, ImportMode.Panel); + ImportRows.LogRejected(_logger, "product", result); } - private async Task Import(ProductDto productDto) + /// Row API: an unknown Id is rejected, a row without Id is matched by Sku + public Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) { - var product = await _productService.GetProductById(productDto.Id); - product ??= await _productService.GetProductBySku(productDto.Sku); + return Import(rows, dryRun, ImportMode.Row, cancellationToken); + } - var isNew = product == null; + /// + /// Row: unknown Id rejected, Sku matching only when no Id is given. Panel (XLSX): by Id, then by Sku; an unknown Id + /// creates the product with that Id. + /// + private enum ImportMode + { + Row, + Panel + } + + private sealed class PictureBudget + { + public int Downloads; + } + + private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + //the cap protects the row API; the panel's spreadsheet import downloads every picture + var budget = mode == ImportMode.Row ? new PictureBudget() : null; + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun, mode, budget)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, ProductDto dto, bool dryRun, ImportMode mode, + PictureBudget budget) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); - if (product == null) product = productDto.MapTo(); - else productDto.MapTo(product); + Product existing = null; + if (!string.IsNullOrEmpty(dto.Id)) + existing = await _productService.GetProductById(dto.Id); + if (existing == null && (string.IsNullOrEmpty(dto.Id) || mode == ImportMode.Panel)) + existing = await _productService.GetProductBySku(dto.Sku); - if (!ValidProduct(product)) return; + if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) + errors.Add($"Id '{dto.Id}' was not found."); + else if (dto.Name == "") + errors.Add("Name cannot be empty."); + else if (string.IsNullOrEmpty(dto.Name ?? existing?.Name)) + errors.Add("Name is required."); + + errors.AddRange(_htmlGuard.RichTextErrors((nameof(dto.ShortDescription), dto.ShortDescription), + (nameof(dto.FullDescription), dto.FullDescription))); + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName), + (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), + (nameof(dto.MetaTitle), dto.MetaTitle), (nameof(dto.Sku), dto.Sku))); + + await AddReferenceWarnings(dto, warnings); + //the Sku is what a later call matches on; without one the row cannot be found again + if (mode == ImportMode.Row && existing == null && string.IsNullOrEmpty(dto.Sku)) + warnings.Add("Row has no Sku; sending it again creates a duplicate."); + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? dto.Id ?? "", key, errors, warnings); + + //a product found by Sku keeps its own id (Panel: the old code mapped the foreign Id over the found product) + if (existing != null) dto.Id = existing.Id; + var product = existing; + var isNew = product == null; + if (product == null) + { + product = dto.MapTo(); + if (mode == ImportMode.Row) ApplyCreateDefaults(dto, product); + } + else + { + dto.MapTo(product); + } + + if (!ValidProduct(product)) + { + errors.Add("Name is required."); + return new ImportRowResult(row, ImportRowStatus.Rejected, "", key, errors, warnings); + } if (isNew) await _productService.InsertProduct(product); else await _productService.UpdateProduct(product); - await UpdateProductData(productDto, product, isNew); + await UpdateProductData(dto, product, isNew, budget, warnings); + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, product.Id, key, + errors, warnings); } - private async Task UpdateProductData(ProductDto productDto, Product product, bool isNew) + /// + /// Row create: the defaults the admin panel puts on a new product, for every field the row leaves out - without + /// them a product cannot be bought (maximum quantity 0) nor found (not visible individually). Published is not + /// defaulted: a row-created product stays unpublished unless the row says otherwise. + /// + private void ApplyCreateDefaults(ProductDto dto, Product product) + { + if (!dto.VisibleIndividually.HasValue) product.VisibleIndividually = true; + if (!dto.OrderMinimumQuantity.HasValue) product.OrderMinimumQuantity = 1; + if (!dto.OrderMaximumQuantity.HasValue) product.OrderMaximumQuantity = 10000; + if (!dto.NotifyAdminForQuantityBelow.HasValue) product.NotifyAdminForQuantityBelow = 1; + if (!dto.IsShipEnabled.HasValue) product.IsShipEnabled = true; + if (!dto.AllowCustomerReviews.HasValue) product.AllowCustomerReviews = true; + if (dto.TaxCategoryId == null) product.TaxCategoryId = _taxSettings.DefaultTaxCategoryId; + } + + private static string Key(ProductDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.Sku) ? dto.Sku : dto.Id ?? ""; + } + + /// Ids that do not exist are skipped (categories, collections) or cleared (brand); say so + private async Task AddReferenceWarnings(ProductDto dto, List warnings) + { + if (!string.IsNullOrEmpty(dto.BrandId) && await _brandService.GetBrandById(dto.BrandId) == null) + warnings.Add($"BrandId '{dto.BrandId}' was not found and was cleared."); + + foreach (var id in SplitIds(dto.CategoryIds)) + if (await _categoryService.GetCategoryById(id) == null) + warnings.Add($"Category id '{id}' was not found and was skipped."); + + foreach (var id in SplitIds(dto.CollectionIds)) + if (await _collectionService.GetCollectionById(id) == null) + warnings.Add($"Collection id '{id}' was not found and was skipped."); + } + + private static IEnumerable SplitIds(string ids) + { + return string.IsNullOrEmpty(ids) + ? [] + : ids.Split([';'], StringSplitOptions.RemoveEmptyEntries).Select(x => x.Trim()); + } + + private async Task UpdateProductData(ProductDto productDto, Product product, bool isNew, + PictureBudget budget, List warnings) { await UpdateProductDataLayout(product); await UpdateProductDataDeliveryDate(product); @@ -116,7 +274,7 @@ private async Task UpdateProductData(ProductDto productDto, Product product, boo await PrepareProductCollections(product, productDto.CollectionIds); //pictures - await PrepareProductPictures(productDto, product, isNew); + await PrepareProductPictures(productDto, product, isNew, budget, warnings); } private async Task UpdateProductDataLayout(Product product) @@ -209,22 +367,32 @@ private async Task PrepareProductCollections(Product product, string collectionI } } - private async Task PrepareProductPictures(ProductDto productDto, Product product, bool isNew) + private async Task PrepareProductPictures(ProductDto productDto, Product product, bool isNew, + PictureBudget budget, List warnings) { - var picture1 = productDto.Picture1; - var picture2 = productDto.Picture2; - var picture3 = productDto.Picture3; + //PictureUrls, when sent, replaces Picture1..3 + IEnumerable urls = productDto.PictureUrls is { Count: > 0 } + ? productDto.PictureUrls + : new[] { productDto.Picture1, productDto.Picture2, productDto.Picture3 }; - foreach (var pictureUrl in new[] { picture1, picture2, picture3 }) + foreach (var pictureUrl in urls) { if (string.IsNullOrEmpty(pictureUrl)) continue; - //only http(s) URLs to public or explicitly allowed hosts are downloaded - a local path would let the - //import read any file on the server - var image = await DownloadUrl.DownloadImage(pictureUrl, _securityConfig.PictureImportAllowedPrivateHosts); + if (budget is { Downloads: >= MaxPictureDownloadsPerBatch }) + { + warnings.Add($"Picture '{pictureUrl}' was not downloaded: picture limit for one batch reached."); + continue; + } + + if (budget != null) budget.Downloads++; + var image = await _downloadImage(pictureUrl); if (image == null) + { + warnings.Add($"Picture '{pictureUrl}' could not be downloaded."); continue; + } var pictureAlreadyExists = false; if (!isNew) diff --git a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs index 10eb3134db..056f8ac977 100644 --- a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs @@ -109,6 +109,8 @@ private static void RegisterExportImport(IServiceCollection serviceCollection) serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped(); - serviceCollection.AddScoped, ProductImportDataObject>(); + serviceCollection.AddScoped(); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); + serviceCollection.AddScoped>(sp => sp.GetRequiredService()); } } \ No newline at end of file diff --git a/src/Business/Grand.Business.Core/Dto/ProductDto.cs b/src/Business/Grand.Business.Core/Dto/ProductDto.cs index fcef42ab9a..e6f13ee3d8 100644 --- a/src/Business/Grand.Business.Core/Dto/ProductDto.cs +++ b/src/Business/Grand.Business.Core/Dto/ProductDto.cs @@ -116,4 +116,7 @@ public class ProductDto public string Picture1 { get; set; } public string Picture2 { get; set; } public string Picture3 { get; set; } + + /// Row import only: when present it replaces Picture1..3 + public IList PictureUrls { get; set; } } \ No newline at end of file diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs index 502e991fe0..b70354c24b 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs @@ -13,11 +13,15 @@ using Grand.Business.Core.Interfaces.Checkout.Shipping; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Seo; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Business.Core.Utilities.System; using Grand.Business.Core.Interfaces.Storage; using Grand.Data; using Grand.Data.Tests.MongoDb; using Grand.Domain; using Grand.Domain.Catalog; +using Grand.Domain.Common; +using Grand.Domain.Media; using Grand.Domain.Customers; using Grand.Domain.Directory; using Grand.Domain.Localization; @@ -29,9 +33,11 @@ using Grand.Infrastructure.Caching; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Mapper; +using Grand.Infrastructure.Security; using Grand.Infrastructure.Tests.Caching; using Grand.Infrastructure.TypeSearch; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -62,10 +68,20 @@ public class ProductImportDataObjectTests private Mock _warehouseServiceMock; private Mock _workContextMock; private ISeNameService _seNameService; + private SecurityConfig _securityConfig; + private TaxSettings _taxSettings; + private Mock> _loggerMock; + private List _downloadedUrls; + private Func _downloadResult; + [TestInitialize] public void Init() { InitAutoMapper(); + _securityConfig = new SecurityConfig(); + _taxSettings = new TaxSettings { DefaultTaxCategoryId = "tax-default" }; + _downloadedUrls = new List(); + _downloadResult = _ => new DownloadedImage([1, 2, 3], "image/png"); _repository = new MongoDBRepositoryTest(); @@ -100,7 +116,15 @@ public void Init() _collectionServiceMock.Object, _productCollectionServiceMock.Object, _seNameService, - new SecurityConfig()); + _securityConfig, + _taxSettings, + new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), + (_loggerMock = new Mock>()).Object, + url => + { + _downloadedUrls.Add(url); + return Task.FromResult(_downloadResult(url)); + }); } [TestMethod] @@ -267,6 +291,482 @@ public async Task ExecuteTest_Import_Insert_Update() Assert.IsFalse(_repository.Table.FirstOrDefault(x => x.Id == product3.Id).Published); } + private void SetupLookups() + { + _productLayoutServiceMock.Setup(c => c.GetProductLayoutById(It.IsAny())) + .Returns(Task.FromResult(new ProductLayout())); + _productLayoutServiceMock.Setup(c => c.GetAllProductLayouts()) + .Returns(Task.FromResult>(new List { new() })); + _languageServiceMock.Setup(c => c.GetAllLanguages(It.IsAny(), It.IsAny())) + .Returns(Task.FromResult>(new List())); + _slugServiceMock.Setup(c => c.GetBySlug(It.IsAny())) + .Returns(Task.FromResult(new EntityUrl { Slug = "slug", EntityName = "Product" })); + _pictureServiceMock.Setup(c => c.InsertPicture(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .ReturnsAsync(() => new Picture { Id = Guid.NewGuid().ToString() }); + } + + [TestMethod] + public async Task Import_NewRow_IsCreatedWithId() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { new() { Name = "new one", Sku = "S1" } }, false); + + Assert.AreEqual(1, result.Created); + var row = result.Rows[0]; + Assert.AreEqual(1, row.Row); + Assert.AreEqual(ImportRowStatus.Created, row.Status); + Assert.AreEqual("new one", row.Key); + Assert.AreEqual(row.Id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Import_ExistingBySku_IsUpdated() + { + SetupLookups(); + var existing = new Product { Name = "old", Sku = "SKU-1" }; + await _productService.InsertProduct(existing); + + var result = await _productImportDataObject.Import( + new List { new() { Sku = "SKU-1", Name = "renamed" } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(existing.Id, result.Rows[0].Id); + Assert.AreEqual("renamed", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_UnknownId_IsRejected() + { + SetupLookups(); + + var result = await _productImportDataObject.Import( + new List { new() { Id = "0123456789abcdef01234567", Name = "x" } }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("was not found"))); + Assert.IsFalse(_repository.Table.Any()); + } + + [TestMethod] + public async Task Execute_UnknownId_StillCreatesProductWithThatId() + { + SetupLookups(); + var id = "0123456789abcdef01234567"; + + await _productImportDataObject.Execute(new List { new() { Id = id, Name = "panel" } }); + + Assert.AreEqual(id, _repository.Table.Single().Id); + } + + [TestMethod] + public async Task Execute_UnknownId_FallsBackToSku() + { + SetupLookups(); + var existing = new Product { Name = "old", Sku = "SKU-9" }; + await _productService.InsertProduct(existing); + + await _productImportDataObject.Execute( + new List { new() { Id = "0123456789abcdef01234567", Sku = "SKU-9", Name = "by sku" } }); + + Assert.AreEqual("by sku", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_PartialUpdate_KeepsNameAndDescription() + { + SetupLookups(); + var existing = new Product { Name = "keep me", FullDescription = "

keep

", Sku = "SKU-2", Price = 1 }; + await _productService.InsertProduct(existing); + + var result = await _productImportDataObject.Import( + new List { new() { Sku = "SKU-2", Price = 42 } }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + var saved = _repository.Table.Single(); + Assert.AreEqual("keep me", saved.Name); + Assert.AreEqual("

keep

", saved.FullDescription); + Assert.AreEqual(42, saved.Price); + } + + [TestMethod] + public async Task Import_NewRowWithoutName_IsRejected() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { new() { Sku = "NONAME" } }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.IsFalse(_repository.Table.Any()); + } + + [TestMethod] + public async Task Import_EmptyName_IsRejectedAlsoInDryRun() + { + SetupLookups(); + var existing = new Product { Name = "old", Sku = "SKU-3" }; + await _productService.InsertProduct(existing); + + var result = await _productImportDataObject.Import( + new List { new() { Sku = "SKU-3", Name = "" } }, true); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.AreEqual("old", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Import_IframeInFullDescription_RejectsRowAndContinues() + { + SetupLookups(); + var rows = new List { + new() { Name = "bad", FullDescription = "" }, + new() { Name = "good" } + }; + + var result = await _productImportDataObject.Import(rows, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("FullDescription"))); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); + Assert.AreEqual("good", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Execute_ScriptInFullDescription_IsNotImported() + { + SetupLookups(); + + await _productImportDataObject.Execute(new List { + new() { Name = "bad", FullDescription = "" } + }); + + Assert.IsFalse(_repository.Table.Any()); + } + + [TestMethod] + public async Task Import_UnknownCategoryAndCollection_WarnsAndStillSaves() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "p", CategoryIds = "nocat", CollectionIds = "nocol" } + }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Warnings.Any(w => w.Contains("nocat"))); + Assert.IsTrue(result.Rows[0].Warnings.Any(w => w.Contains("nocol"))); + Assert.AreEqual(1, _repository.Table.Count()); + } + + [TestMethod] + public async Task Import_UnknownBrand_Warns() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "p", BrandId = "nobrand" } + }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Warnings.Any(w => w.Contains("nobrand"))); + Assert.AreEqual("", _repository.Table.Single().BrandId); + } + + [TestMethod] + public async Task Import_DryRun_WritesAndDownloadsNothing() + { + SetupLookups(); + var existing = new Product { Name = "old", Sku = "SKU-4" }; + await _productService.InsertProduct(existing); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "fresh", Picture1 = "https://example.com/a.png" }, + new() { Sku = "SKU-4", Name = "renamed", PictureUrls = ["https://example.com/b.png"] } + }, true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[1].Status); + Assert.AreEqual(1, _repository.Table.Count()); + Assert.AreEqual("old", _repository.Table.Single().Name); + Assert.AreEqual(0, _downloadedUrls.Count); + _slugServiceMock.Verify(c => c.SaveSlug(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _pictureServiceMock.Verify(c => c.InsertPicture(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Import_PictureUrls_ReplacePicture1To3() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { + new() { + Name = "p", Picture1 = "https://example.com/old.png", + PictureUrls = ["https://example.com/x.png", "https://example.com/y.png"] + } + }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + CollectionAssert.AreEqual(new[] { "https://example.com/x.png", "https://example.com/y.png" }, _downloadedUrls); + } + + [TestMethod] + public async Task Import_Picture1To3_AreDownloadedWhenNoPictureUrls() + { + SetupLookups(); + + await _productImportDataObject.Import(new List { + new() { Name = "p", Picture1 = "https://example.com/1.png", Picture3 = "https://example.com/3.png" } + }, false); + + CollectionAssert.AreEqual(new[] { "https://example.com/1.png", "https://example.com/3.png" }, _downloadedUrls); + } + + [TestMethod] + public async Task Import_FailedPictureDownload_Warns() + { + SetupLookups(); + _downloadResult = _ => null; + + var result = await _productImportDataObject.Import(new List { + new() { Name = "p", Picture1 = "https://example.com/dead.png" } + }, false); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Warnings.Any(w => w.Contains("dead.png"))); + } + + [TestMethod] + public async Task Import_PictureCap_StopsDownloadingAfter20AndWarns() + { + SetupLookups(); + var rows = new List(); + for (var i = 0; i < 5; i++) + rows.Add(new ProductDto { + Name = $"p{i}", + //a Sku keeps the "no Sku" warning out of the picture warnings checked below + Sku = $"S{i}", + PictureUrls = Enumerable.Range(0, 5).Select(n => $"https://example.com/{i}-{n}.png").ToList() + }); + + var result = await _productImportDataObject.Import(rows, false); + + Assert.AreEqual(20, _downloadedUrls.Count); + Assert.IsFalse(result.Rows[3].Warnings.Any()); + Assert.AreEqual(5, result.Rows[4].Warnings.Count(w => w.Contains("picture limit for one batch reached"))); + Assert.AreEqual(5, result.Created); + } + + [TestMethod] + public async Task Execute_MoreThan20Pictures_AreAllDownloaded() + { + SetupLookups(); + var rows = new List(); + for (var i = 0; i < 5; i++) + rows.Add(new ProductDto { + Name = $"p{i}", + PictureUrls = Enumerable.Range(0, 5).Select(n => $"https://example.com/{i}-{n}.png").ToList() + }); + + await _productImportDataObject.Execute(rows); + + Assert.AreEqual(25, _downloadedUrls.Count); + } + + [TestMethod] + public async Task Import_MarkupInPlainFields_RejectsRow() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "bold" }, + new() { Name = "ok", Sku = "" } + }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.IsTrue(result.Rows[0].Errors.Any(e => e.Contains("Name"))); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.IsTrue(result.Rows[1].Errors.Any(e => e.Contains("Sku"))); + Assert.IsFalse(_repository.Table.Any()); + } + + [TestMethod] + public async Task Import_EmptyName_GivesOneError() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { new() { Name = "" } }, false); + + Assert.AreEqual(1, result.Rows[0].Errors.Count); + } + + [TestMethod] + public async Task Import_StoredNameEmptyAndNoNameSent_RejectedInDryRunAndRealRun() + { + SetupLookups(); + await _productService.InsertProduct(new Product { Name = "", Sku = "SKU-E" }); + var rows = new List { new() { Sku = "SKU-E", Price = 5 } }; + + var dry = await _productImportDataObject.Import(rows, true); + var real = await _productImportDataObject.Import(rows, false); + + Assert.AreEqual(ImportRowStatus.Rejected, dry.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, real.Rows[0].Status); + } + + [TestMethod] + public async Task Import_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + SetupLookups(); + using var cts = new CancellationTokenSource(); + //the layout lookup runs while row 1 is saved + _productLayoutServiceMock.Setup(c => c.GetAllProductLayouts()) + .Callback(() => cts.Cancel()) + .Returns(Task.FromResult>(new List { new() })); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "one", Sku = "S1" }, new() { Name = "two", Sku = "S2" }, new() { Name = "three", Sku = "S3" } + }, false, cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual("three", result.Rows[2].Key); + Assert.AreEqual("Not processed: the batch time limit was reached; send these rows again.", + result.Rows[1].Errors.Single()); + Assert.AreEqual("one", _repository.Table.Single().Name); + } + + [TestMethod] + public async Task Execute_RejectedRows_LogsOneWarningWithRowNumbersAndReasons() + { + SetupLookups(); + + await _productImportDataObject.Execute(new List { + new() { Name = "good", Sku = "G" }, + new() { Name = "bad", Sku = "B", FullDescription = "" } + }); + + Assert.AreEqual("good", _repository.Table.Single().Name); + _loggerMock.Verify(l => l.Log(LogLevel.Warning, It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains("row 2") && v.ToString()!.Contains("FullDescription")), + It.IsAny(), It.IsAny>()), Times.Once); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_NewRowWithoutSku_WarnsAboutDuplicates(bool dryRun) + { + SetupLookups(); + var existing = new Product { Name = "old" }; + await _productService.InsertProduct(existing); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "no sku" }, + new() { Name = "with sku", Sku = "S1" }, + new() { Id = existing.Id, Name = "renamed" } + }, dryRun); + + const string warning = "Row has no Sku; sending it again creates a duplicate."; + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.Contains(warning, result.Rows[0].Warnings); + Assert.DoesNotContain(warning, result.Rows[1].Warnings); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[2].Status); + Assert.DoesNotContain(warning, result.Rows[2].Warnings); + } + + [TestMethod] + public async Task Import_NewRow_GetsThePanelCreateDefaults() + { + SetupLookups(); + _taxServiceMock.Setup(c => c.GetTaxCategoryById("tax-default")).ReturnsAsync(new TaxCategory { Id = "tax-default" }); + + await _productImportDataObject.Import(new List { new() { Name = "buyable", Sku = "B1" } }, false); + + var product = _repository.Table.Single(); + Assert.IsTrue(product.VisibleIndividually); + Assert.AreEqual(1, product.OrderMinimumQuantity); + Assert.AreEqual(10000, product.OrderMaximumQuantity); + Assert.AreEqual(1, product.NotifyAdminForQuantityBelow); + Assert.IsTrue(product.IsShipEnabled); + Assert.IsTrue(product.AllowCustomerReviews); + Assert.AreEqual("tax-default", product.TaxCategoryId); + Assert.IsFalse(product.Published); + } + + [TestMethod] + public async Task Import_NewRow_ExplicitValuesWinOverCreateDefaults() + { + SetupLookups(); + _taxServiceMock.Setup(c => c.GetTaxCategoryById("tax-own")).ReturnsAsync(new TaxCategory { Id = "tax-own" }); + + await _productImportDataObject.Import(new List { + new() { + Name = "own", Sku = "B2", VisibleIndividually = false, OrderMinimumQuantity = 2, + OrderMaximumQuantity = 5, NotifyAdminForQuantityBelow = 3, IsShipEnabled = false, + AllowCustomerReviews = false, TaxCategoryId = "tax-own", Published = true + } + }, false); + + var product = _repository.Table.Single(); + Assert.IsFalse(product.VisibleIndividually); + Assert.AreEqual(2, product.OrderMinimumQuantity); + Assert.AreEqual(5, product.OrderMaximumQuantity); + Assert.AreEqual(3, product.NotifyAdminForQuantityBelow); + Assert.IsFalse(product.IsShipEnabled); + Assert.IsFalse(product.AllowCustomerReviews); + Assert.AreEqual("tax-own", product.TaxCategoryId); + Assert.IsTrue(product.Published); + } + + [TestMethod] + public async Task Import_ExistingRow_DoesNotGetCreateDefaults() + { + SetupLookups(); + await _productService.InsertProduct(new Product { Name = "old", Sku = "B3" }); + + await _productImportDataObject.Import(new List { new() { Sku = "B3", Name = "renamed" } }, false); + + var product = _repository.Table.Single(); + Assert.IsFalse(product.VisibleIndividually); + Assert.AreEqual(0, product.OrderMaximumQuantity); + Assert.IsNull(product.TaxCategoryId); + } + + [TestMethod] + public async Task Execute_PanelCreate_DoesNotGetRowCreateDefaults() + { + SetupLookups(); + _taxServiceMock.Setup(c => c.GetTaxCategoryById("tax-default")).ReturnsAsync(new TaxCategory { Id = "tax-default" }); + + await _productImportDataObject.Execute(new List { new() { Name = "panel", Sku = "P1" } }); + + var product = _repository.Table.Single(); + Assert.IsFalse(product.VisibleIndividually); + Assert.AreEqual(0, product.OrderMinimumQuantity); + Assert.AreEqual(0, product.OrderMaximumQuantity); + Assert.IsFalse(product.IsShipEnabled); + Assert.IsTrue(string.IsNullOrEmpty(product.TaxCategoryId)); + } + + [TestMethod] + public async Task Import_DryRun_NewRowWithDefaultsWritesNothing() + { + SetupLookups(); + + var result = await _productImportDataObject.Import(new List { new() { Name = "dry", Sku = "D1" } }, true); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual("", result.Rows[0].Id); + Assert.IsFalse(_repository.Table.Any()); + } + private void InitAutoMapper() { var typeSearcher = new TypeSearcher(); From c83c374ed941514f28db249c94f7a9505e2bc8d1 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Thu, 8 Oct 2026 14:56:03 +0200 Subject: [PATCH 3/6] Catalog import: specification and product attributes with option merge --- .../ProductAttributeImportDataObject.cs | 154 ++++++++ .../SpecificationAttributeImportDataObject.cs | 151 ++++++++ .../Startup/StartupApplication.cs | 2 + .../Dto/ProductAttributeDto.cs | 22 ++ .../Dto/SpecificationAttributeDto.cs | 19 + .../AttributeImportDataObjectTests.cs | 346 ++++++++++++++++++ 6 files changed, 694 insertions(+) create mode 100644 src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs create mode 100644 src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs create mode 100644 src/Business/Grand.Business.Core/Dto/ProductAttributeDto.cs create mode 100644 src/Business/Grand.Business.Core/Dto/SpecificationAttributeDto.cs create mode 100644 src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs new file mode 100644 index 0000000000..3bdf232ffc --- /dev/null +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs @@ -0,0 +1,154 @@ +using Grand.Business.Core.Dto; +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Catalog.Products; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Catalog; +using Grand.Domain.Seo; + +namespace Grand.Business.Catalog.Services.ExportImport; + +/// Row API only: there is no XLSX import for product attributes +public class ProductAttributeImportDataObject : IRowImport +{ + private readonly ImportHtmlGuard _htmlGuard; + private readonly IProductAttributeService _productAttributeService; + private readonly SeoSettings _seoSettings; + + public ProductAttributeImportDataObject(IProductAttributeService productAttributeService, + SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + { + _productAttributeService = productAttributeService; + _seoSettings = seoSettings; + _htmlGuard = htmlGuard; + } + + public async Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, ProductAttributeDto dto, bool dryRun) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + + var (existing, matchedByName) = await Find(dto); + if (matchedByName) + warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); + if (!string.IsNullOrEmpty(dto.Id) && existing == null) + errors.Add($"Id '{dto.Id}' was not found."); + else if (existing == null && string.IsNullOrEmpty(dto.Name)) + errors.Add("Name is required for a new product attribute."); + if (dto.Name == "") + errors.Add("Name cannot be empty."); + + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName))); + errors.AddRange(_htmlGuard.RichTextErrors((nameof(dto.Description), dto.Description))); + foreach (var value in dto.Values ?? []) + { + if (string.IsNullOrEmpty(value?.Name)) + { + errors.Add("Value Name is required."); + continue; + } + + errors.AddRange(_htmlGuard.PlainTextErrors(("Value Name", value.Name))); + } + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? "", key, errors, warnings); + + var isNew = existing == null; + var attribute = existing ?? new ProductAttribute { Name = dto.Name }; + if (!string.IsNullOrEmpty(dto.Name)) attribute.Name = dto.Name; + if (dto.Description != null) attribute.Description = dto.Description; + if (isNew || !string.IsNullOrEmpty(dto.SeName)) + attribute.SeName = GetSeName(string.IsNullOrEmpty(dto.SeName) ? attribute.Name : dto.SeName); + + //a repeated name in the same row matches the value just added, so it collapses into it + foreach (var valueDto in dto.Values ?? []) + { + var value = attribute.PredefinedProductAttributeValues + .FirstOrDefault(v => string.Equals(v.Name, valueDto.Name, StringComparison.OrdinalIgnoreCase)); + if (value == null) + { + value = new PredefinedProductAttributeValue { Name = valueDto.Name }; + attribute.PredefinedProductAttributeValues.Add(value); + } + + if (valueDto.PriceAdjustment.HasValue) value.PriceAdjustment = valueDto.PriceAdjustment.Value; + if (valueDto.WeightAdjustment.HasValue) value.WeightAdjustment = valueDto.WeightAdjustment.Value; + if (valueDto.Cost.HasValue) value.Cost = valueDto.Cost.Value; + if (valueDto.IsPreSelected.HasValue) value.IsPreSelected = valueDto.IsPreSelected.Value; + if (valueDto.DisplayOrder.HasValue) value.DisplayOrder = valueDto.DisplayOrder.Value; + } + + if (isNew) await _productAttributeService.InsertProductAttribute(attribute); + else await _productAttributeService.UpdateProductAttribute(attribute); + + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, attribute.Id, key, + errors, warnings); + } + + private static string Key(ProductAttributeDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.SeName) ? dto.SeName : dto.Id ?? ""; + } + + /// + /// By Id, else by SeName normalized as a new attribute's is, else (best effort) by the SeName its Name would get, + /// accepted only when the stored name is the same (any case) + /// + private async Task<(ProductAttribute attribute, bool matchedByName)> Find(ProductAttributeDto dto) + { + if (!string.IsNullOrEmpty(dto.Id)) + return (await _productAttributeService.GetProductAttributeById(dto.Id), false); + + if (!string.IsNullOrEmpty(dto.SeName)) + return (await FindBySeName(GetSeName(dto.SeName)), false); + + if (string.IsNullOrEmpty(dto.Name)) + return (null, false); + + var found = await FindBySeName(GetSeName(dto.Name)); + return found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase) + ? (found, true) + : (null, false); + } + + //no lookup by SeName exists: load all and match + private async Task FindBySeName(string seName) + { + if (string.IsNullOrEmpty(seName)) + return null; + + var all = await _productAttributeService.GetAllProductAttributes(); + return all.FirstOrDefault(a => string.Equals(a.SeName, seName, StringComparison.OrdinalIgnoreCase)); + } + + //same normalization as the panel's product attribute controller + private string GetSeName(string name) + { + return SeoExtensions.GetSeName(name, _seoSettings.ConvertNonWesternChars, _seoSettings.AllowUnicodeCharsInUrls, + _seoSettings.SeoCharConversion); + } +} diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs new file mode 100644 index 0000000000..f90a529506 --- /dev/null +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs @@ -0,0 +1,151 @@ +using Grand.Business.Core.Dto; +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Catalog.Products; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Catalog; +using Grand.Domain.Seo; + +namespace Grand.Business.Catalog.Services.ExportImport; + +/// Row API only: there is no XLSX import for specification attributes +public class SpecificationAttributeImportDataObject : IRowImport +{ + private readonly ImportHtmlGuard _htmlGuard; + private readonly SeoSettings _seoSettings; + private readonly ISpecificationAttributeService _specificationAttributeService; + + public SpecificationAttributeImportDataObject(ISpecificationAttributeService specificationAttributeService, + SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + { + _specificationAttributeService = specificationAttributeService; + _seoSettings = seoSettings; + _htmlGuard = htmlGuard; + } + + public async Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, SpecificationAttributeDto dto, bool dryRun) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + + var (existing, matchedByName) = await Find(dto); + if (matchedByName) + warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); + if (!string.IsNullOrEmpty(dto.Id) && existing == null) + errors.Add($"Id '{dto.Id}' was not found."); + else if (existing == null && string.IsNullOrEmpty(dto.Name)) + errors.Add("Name is required for a new specification attribute."); + if (dto.Name == "") + errors.Add("Name cannot be empty."); + + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(dto.Name), dto.Name), (nameof(dto.SeName), dto.SeName))); + foreach (var option in dto.Options ?? []) + { + if (string.IsNullOrEmpty(option?.Name)) + { + errors.Add("Option Name is required."); + continue; + } + + errors.AddRange(_htmlGuard.PlainTextErrors(("Option Name", option.Name), + ("Option ColorSquaresRgb", option.ColorSquaresRgb))); + } + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); + + if (dryRun) + return new ImportRowResult(row, existing == null ? ImportRowStatus.Created : ImportRowStatus.Updated, + existing?.Id ?? "", key, errors, warnings); + + var isNew = existing == null; + var attribute = existing ?? new SpecificationAttribute { Name = dto.Name }; + if (!string.IsNullOrEmpty(dto.Name)) attribute.Name = dto.Name; + if (dto.DisplayOrder.HasValue) attribute.DisplayOrder = dto.DisplayOrder.Value; + if (isNew || !string.IsNullOrEmpty(dto.SeName)) + attribute.SeName = GetSeName(string.IsNullOrEmpty(dto.SeName) ? attribute.Name : dto.SeName); + + //a repeated name in the same row matches the option just added, so it collapses into it + foreach (var optionDto in dto.Options ?? []) + { + var option = attribute.SpecificationAttributeOptions + .FirstOrDefault(o => string.Equals(o.Name, optionDto.Name, StringComparison.OrdinalIgnoreCase)); + if (option == null) + { + option = new SpecificationAttributeOption { + Name = optionDto.Name, + SeName = GetSeName(optionDto.Name) + }; + attribute.SpecificationAttributeOptions.Add(option); + } + + if (optionDto.ColorSquaresRgb != null) option.ColorSquaresRgb = optionDto.ColorSquaresRgb; + if (optionDto.DisplayOrder.HasValue) option.DisplayOrder = optionDto.DisplayOrder.Value; + } + + if (isNew) await _specificationAttributeService.InsertSpecificationAttribute(attribute); + else await _specificationAttributeService.UpdateSpecificationAttribute(attribute); + + return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, attribute.Id, key, + errors, warnings); + } + + private static string Key(SpecificationAttributeDto dto) + { + return !string.IsNullOrEmpty(dto.Name) ? dto.Name : !string.IsNullOrEmpty(dto.SeName) ? dto.SeName : dto.Id ?? ""; + } + + /// + /// By Id, else by SeName normalized as a new attribute's is, else (best effort) by the SeName its Name would get, + /// accepted only when the stored name is the same (any case) + /// + private async Task<(SpecificationAttribute attribute, bool matchedByName)> Find(SpecificationAttributeDto dto) + { + if (!string.IsNullOrEmpty(dto.Id)) + return (await _specificationAttributeService.GetSpecificationAttributeById(dto.Id), false); + + if (!string.IsNullOrEmpty(dto.SeName)) + return (await FindBySeName(GetSeName(dto.SeName)), false); + + if (string.IsNullOrEmpty(dto.Name)) + return (null, false); + + var found = await FindBySeName(GetSeName(dto.Name)); + return found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase) + ? (found, true) + : (null, false); + } + + private async Task FindBySeName(string seName) + { + return string.IsNullOrEmpty(seName) + ? null + : await _specificationAttributeService.GetSpecificationAttributeBySeName(seName); + } + + //same normalization as the panel's specification attribute controller + private string GetSeName(string name) + { + return SeoExtensions.GetSeName(name, _seoSettings.ConvertNonWesternChars, _seoSettings.AllowUnicodeCharsInUrls, + _seoSettings.SeoCharConversion); + } +} diff --git a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs index 056f8ac977..ed93673ecb 100644 --- a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs @@ -109,6 +109,8 @@ private static void RegisterExportImport(IServiceCollection serviceCollection) serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped(); + serviceCollection.AddScoped, SpecificationAttributeImportDataObject>(); + serviceCollection.AddScoped, ProductAttributeImportDataObject>(); serviceCollection.AddScoped(); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); diff --git a/src/Business/Grand.Business.Core/Dto/ProductAttributeDto.cs b/src/Business/Grand.Business.Core/Dto/ProductAttributeDto.cs new file mode 100644 index 0000000000..2927f886e8 --- /dev/null +++ b/src/Business/Grand.Business.Core/Dto/ProductAttributeDto.cs @@ -0,0 +1,22 @@ +namespace Grand.Business.Core.Dto; + +public class ProductAttributeDto +{ + public string Id { get; set; } + public string Name { get; set; } + public string SeName { get; set; } + public string Description { get; set; } + + /// Predefined values, merged by name (case-insensitive): missing values are added, provided fields updated, none deleted + public IList Values { get; set; } +} + +public class ProductAttributeValueDto +{ + public string Name { get; set; } + public double? PriceAdjustment { get; set; } + public double? WeightAdjustment { get; set; } + public double? Cost { get; set; } + public bool? IsPreSelected { get; set; } + public int? DisplayOrder { get; set; } +} diff --git a/src/Business/Grand.Business.Core/Dto/SpecificationAttributeDto.cs b/src/Business/Grand.Business.Core/Dto/SpecificationAttributeDto.cs new file mode 100644 index 0000000000..98520e2e6c --- /dev/null +++ b/src/Business/Grand.Business.Core/Dto/SpecificationAttributeDto.cs @@ -0,0 +1,19 @@ +namespace Grand.Business.Core.Dto; + +public class SpecificationAttributeDto +{ + public string Id { get; set; } + public string Name { get; set; } + public string SeName { get; set; } + public int? DisplayOrder { get; set; } + + /// Merged by name (case-insensitive): missing options are added, provided fields updated, none deleted + public IList Options { get; set; } +} + +public class SpecificationAttributeOptionDto +{ + public string Name { get; set; } + public string ColorSquaresRgb { get; set; } + public int? DisplayOrder { get; set; } +} diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs new file mode 100644 index 0000000000..6a3e0c5550 --- /dev/null +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs @@ -0,0 +1,346 @@ +using Grand.Business.Catalog.Services.ExportImport; +using Grand.Business.Core.Dto; +using Grand.Business.Core.Interfaces.Catalog.Products; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Catalog; +using Grand.Domain.Seo; +using Grand.Infrastructure.Configuration; +using Grand.Infrastructure.Security; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Catalog.Tests.Services.ExportImport; + +[TestClass] +public class AttributeImportDataObjectTests +{ + private Mock _productAttributeService; + private ProductAttributeImportDataObject _productImport; + private List _productAttributes; + private Mock _specificationAttributeService; + private SpecificationAttributeImportDataObject _specificationImport; + private List _specificationAttributes; + private int _writes; + + [TestInitialize] + public void Init() + { + _writes = 0; + _specificationAttributes = []; + _productAttributes = []; + var securityConfig = new SecurityConfig(); + var guard = new ImportHtmlGuard(new HtmlSanitizationService(securityConfig), securityConfig); + + _specificationAttributeService = new Mock(); + _specificationAttributeService.Setup(s => s.GetSpecificationAttributeById(It.IsAny())) + .ReturnsAsync((string id) => _specificationAttributes.FirstOrDefault(a => a.Id == id)); + _specificationAttributeService.Setup(s => s.GetSpecificationAttributeBySeName(It.IsAny())) + .ReturnsAsync((string s) => _specificationAttributes.FirstOrDefault(a => a.SeName == s)); + _specificationAttributeService.Setup(s => s.InsertSpecificationAttribute(It.IsAny())) + .Callback((SpecificationAttribute a) => { _writes++; _specificationAttributes.Add(a); }) + .Returns(Task.CompletedTask); + _specificationAttributeService.Setup(s => s.UpdateSpecificationAttribute(It.IsAny())) + .Callback(() => _writes++).Returns(Task.CompletedTask); + _specificationImport = + new SpecificationAttributeImportDataObject(_specificationAttributeService.Object, new SeoSettings(), guard); + + _productAttributeService = new Mock(); + _productAttributeService.Setup(s => s.GetProductAttributeById(It.IsAny())) + .ReturnsAsync((string id) => _productAttributes.FirstOrDefault(a => a.Id == id)); + _productAttributeService.Setup(s => s.GetAllProductAttributes(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(() => new Grand.Domain.PagedList(_productAttributes, 0, int.MaxValue)); + _productAttributeService.Setup(s => s.InsertProductAttribute(It.IsAny())) + .Callback((ProductAttribute a) => { _writes++; _productAttributes.Add(a); }).Returns(Task.CompletedTask); + _productAttributeService.Setup(s => s.UpdateProductAttribute(It.IsAny())) + .Callback(() => _writes++).Returns(Task.CompletedTask); + _productImport = new ProductAttributeImportDataObject(_productAttributeService.Object, new SeoSettings(), guard); + } + + [TestMethod] + public async Task Specification_Create_WithOptions() + { + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { + Name = "Color Family", DisplayOrder = 3, + Options = [new() { Name = "Red", ColorSquaresRgb = "#ff0000" }, new() { Name = "Blue", DisplayOrder = 2 }] + } + ], false); + + Assert.AreEqual(1, result.Created); + var created = _specificationAttributes.Single(); + Assert.AreEqual("color-family", created.SeName); + Assert.AreEqual(3, created.DisplayOrder); + Assert.AreEqual(2, created.SpecificationAttributeOptions.Count); + Assert.AreEqual("#ff0000", created.SpecificationAttributeOptions.First(o => o.Name == "Red").ColorSquaresRgb); + Assert.AreEqual(2, created.SpecificationAttributeOptions.First(o => o.Name == "Blue").DisplayOrder); + } + + [TestMethod] + public async Task Specification_UpdateBySeName_AddsOptionAndKeepsOld() + { + var existing = new SpecificationAttribute { Id = "1", Name = "Size", SeName = "size" }; + existing.SpecificationAttributeOptions.Add(new SpecificationAttributeOption { Name = "Small", DisplayOrder = 1 }); + existing.SpecificationAttributeOptions.Add(new SpecificationAttributeOption { Name = "Large" }); + _specificationAttributes.Add(existing); + + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { + SeName = "size", Options = [new() { Name = "SMALL" }, new() { Name = "Medium" }] + } + ], false); + + Assert.AreEqual(1, result.Updated); + Assert.AreEqual("1", result.Rows[0].Id); + Assert.AreEqual("Size", existing.Name); + Assert.AreEqual(3, existing.SpecificationAttributeOptions.Count); + Assert.AreEqual(1, existing.SpecificationAttributeOptions.First(o => o.Name == "Small").DisplayOrder, + "a field that was not provided is kept"); + Assert.IsTrue(existing.SpecificationAttributeOptions.Any(o => o.Name == "Large")); + Assert.IsTrue(existing.SpecificationAttributeOptions.Any(o => o.Name == "Medium")); + } + + [TestMethod] + public async Task Specification_DuplicateOptionNames_CollapseToOne() + { + await _specificationImport.Import([ + new SpecificationAttributeDto { + Name = "Material", + Options = [new() { Name = "Wool" }, new() { Name = "wool", DisplayOrder = 5 }] + } + ], false); + + var option = _specificationAttributes.Single().SpecificationAttributeOptions.Single(); + Assert.AreEqual("Wool", option.Name); + Assert.AreEqual(5, option.DisplayOrder); + } + + [TestMethod] + public async Task Specification_MarkupInOptionName_Rejected() + { + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { Name = "Ok", Options = [new() { Name = "" }] } + ], false); + + Assert.AreEqual(1, result.Rejected); + Assert.AreEqual(0, _writes); + } + + [TestMethod] + public async Task Specification_UnknownId_Rejected_EmptyNameRejected_NewWithoutNameRejected() + { + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { Id = "nope", Name = "x" }, + new SpecificationAttributeDto { Name = "" }, + new SpecificationAttributeDto { SeName = "unknown" } + ], false); + + Assert.AreEqual(3, result.Rejected); + Assert.AreEqual("Id 'nope' was not found.", result.Rows[0].Errors[0]); + Assert.AreEqual(0, _writes); + } + + [TestMethod] + public async Task Specification_DryRun_WritesNothing() + { + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { Name = "Dry", Options = [new() { Name = "A" }] } + ], true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(1, result.Created); + Assert.AreEqual(0, _writes); + Assert.AreEqual(0, _specificationAttributes.Count); + } + + [TestMethod] + public async Task Product_Create_WithValues() + { + var result = await _productImport.Import([ + new ProductAttributeDto { + Name = "Gift Wrap", Description = "

Nice

", + Values = [new() { Name = "Gold", PriceAdjustment = 2.5, IsPreSelected = true }] + } + ], false); + + Assert.AreEqual(1, result.Created); + var created = _productAttributes.Single(); + Assert.AreEqual("gift-wrap", created.SeName); + var value = created.PredefinedProductAttributeValues.Single(); + Assert.AreEqual(2.5, value.PriceAdjustment); + Assert.IsTrue(value.IsPreSelected); + } + + [TestMethod] + public async Task Product_UpdateBySeName_AddsValueAndKeepsOld() + { + var existing = new ProductAttribute { Id = "7", Name = "Size", SeName = "size" }; + existing.PredefinedProductAttributeValues.Add(new PredefinedProductAttributeValue { Name = "S", Cost = 4 }); + _productAttributes.Add(existing); + + var result = await _productImport.Import([ + new ProductAttributeDto { + SeName = "SIZE", Values = [new() { Name = "s", PriceAdjustment = 1 }, new() { Name = "M" }] + } + ], false); + + Assert.AreEqual(1, result.Updated); + Assert.AreEqual(2, existing.PredefinedProductAttributeValues.Count); + var s = existing.PredefinedProductAttributeValues.First(v => v.Name == "S"); + Assert.AreEqual(1, s.PriceAdjustment); + Assert.AreEqual(4, s.Cost, "a field that was not provided is kept"); + } + + [TestMethod] + public async Task Product_DuplicateValueNames_CollapseToOne() + { + await _productImport.Import([ + new ProductAttributeDto { Name = "A", Values = [new() { Name = "x" }, new() { Name = "X", Cost = 1 }] } + ], false); + + var value = _productAttributes.Single().PredefinedProductAttributeValues.Single(); + Assert.AreEqual(1, value.Cost); + } + + [TestMethod] + public async Task Product_MarkupInValueName_Rejected() + { + var result = await _productImport.Import([ + new ProductAttributeDto { Name = "A", Values = [new() { Name = "" }] } + ], false); + + Assert.AreEqual(1, result.Rejected); + Assert.AreEqual(0, _writes); + } + + [TestMethod] + public async Task Product_UnknownId_Rejected() + { + var result = await _productImport.Import([new ProductAttributeDto { Id = "zz", Name = "x" }], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.AreEqual("Id 'zz' was not found.", result.Rows[0].Errors[0]); + } + + [TestMethod] + public async Task Product_DryRun_WritesNothing() + { + var result = await _productImport.Import([ + new ProductAttributeDto { Name = "Dry", Values = [new() { Name = "A" }] } + ], true); + + Assert.AreEqual(1, result.Created); + Assert.AreEqual(0, _writes); + Assert.AreEqual(0, _productAttributes.Count); + } + + private const string NotProcessed = "Not processed: the batch time limit was reached; send these rows again."; + + [TestMethod] + public async Task Specification_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + using var cts = new CancellationTokenSource(); + _specificationAttributeService.Setup(s => s.InsertSpecificationAttribute(It.IsAny())) + .Callback((SpecificationAttribute a) => { _writes++; _specificationAttributes.Add(a); cts.Cancel(); }) + .Returns(Task.CompletedTask); + + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { Name = "One" }, new SpecificationAttributeDto { Name = "Two" }, + new SpecificationAttributeDto { Name = "Three" } + ], false, cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual("Three", result.Rows[2].Key); + Assert.AreEqual(NotProcessed, result.Rows[1].Errors.Single()); + Assert.AreEqual(1, _writes); + } + + [TestMethod] + public async Task Product_CancelledAfterFirstRow_RestAreRejectedAsNotProcessed() + { + using var cts = new CancellationTokenSource(); + _productAttributeService.Setup(s => s.InsertProductAttribute(It.IsAny())) + .Callback((ProductAttribute a) => { _writes++; _productAttributes.Add(a); cts.Cancel(); }) + .Returns(Task.CompletedTask); + + var result = await _productImport.Import([ + new ProductAttributeDto { Name = "One" }, new ProductAttributeDto { Name = "Two" }, + new ProductAttributeDto { Name = "Three" } + ], false, cts.Token); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[2].Status); + Assert.AreEqual(NotProcessed, result.Rows[2].Errors.Single()); + Assert.AreEqual(1, _writes); + } + + [TestMethod] + public async Task Specification_SeNameIsNormalizedBeforeMatching() + { + _specificationAttributes.Add(new SpecificationAttribute { Id = "1", Name = "Shirt Size", SeName = "shirt-size" }); + + var result = await _specificationImport.Import([new SpecificationAttributeDto { SeName = "Shirt Size" }], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual("1", result.Rows[0].Id); + Assert.AreEqual(1, _specificationAttributes.Count); + } + + [TestMethod] + public async Task Product_SeNameIsNormalizedBeforeMatching() + { + _productAttributes.Add(new ProductAttribute { Id = "7", Name = "Shirt Size", SeName = "shirt-size" }); + + var result = await _productImport.Import([new ProductAttributeDto { SeName = "Shirt Size" }], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual("7", result.Rows[0].Id); + Assert.AreEqual(1, _productAttributes.Count); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Specification_NameOnlyResent_MatchesExistingByName(bool dryRun) + { + _specificationAttributes.Add(new SpecificationAttribute { Id = "1", Name = "Shirt Size", SeName = "shirt-size" }); + + var result = await _specificationImport.Import( + [new SpecificationAttributeDto { Name = "shirt size", Options = [new() { Name = "XL" }] }], dryRun); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual("1", result.Rows[0].Id); + Assert.Contains("Matched existing 'Shirt Size' by name.", result.Rows[0].Warnings); + Assert.AreEqual(1, _specificationAttributes.Count); + Assert.AreEqual(dryRun ? 0 : 1, _specificationAttributes.Single().SpecificationAttributeOptions.Count); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Product_NameOnlyResent_MatchesExistingByName(bool dryRun) + { + _productAttributes.Add(new ProductAttribute { Id = "7", Name = "Gift Wrap", SeName = "gift-wrap" }); + + var result = await _productImport.Import( + [new ProductAttributeDto { Name = "GIFT WRAP", Values = [new() { Name = "Gold" }] }], dryRun); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual("7", result.Rows[0].Id); + Assert.Contains("Matched existing 'Gift Wrap' by name.", result.Rows[0].Warnings); + Assert.AreEqual(1, _productAttributes.Count); + Assert.AreEqual(dryRun ? 0 : 1, _productAttributes.Single().PredefinedProductAttributeValues.Count); + } + + [TestMethod] + public async Task Specification_SlugOfDifferentlyNamedAttribute_IsNotMatchedByName() + { + _specificationAttributes.Add(new SpecificationAttribute { Id = "1", Name = "Shirt-Size", SeName = "shirt-size" }); + + var result = await _specificationImport.Import([new SpecificationAttributeDto { Name = "Shirt Size" }], true); + + Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + } +} From a34cc7ff4d1aab2b014481f53d54911e66399847 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Thu, 8 Oct 2026 14:56:41 +0200 Subject: [PATCH 4/6] Catalog import: product variants - mappings, values and explicit combinations --- .../ProductVariantImportDataObject.cs | 452 +++++++++++++ .../Startup/StartupApplication.cs | 1 + .../Dto/ProductVariantsDto.cs | 57 ++ .../ProductVariantImportDataObjectTests.cs | 618 ++++++++++++++++++ 4 files changed, 1128 insertions(+) create mode 100644 src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs create mode 100644 src/Business/Grand.Business.Core/Dto/ProductVariantsDto.cs create mode 100644 src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs new file mode 100644 index 0000000000..69ab447efe --- /dev/null +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs @@ -0,0 +1,452 @@ +using Grand.Business.Core.Dto; +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Catalog.Products; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Catalog; +using Grand.Domain.Common; +using Grand.Domain.Seo; + +namespace Grand.Business.Catalog.Services.ExportImport; + +/// +/// Row API only: assigns catalogue product attributes to an existing product (mappings and their values) and +/// creates or updates an explicit list of attribute combinations. Nothing is deleted; the product is never created. +/// A dry run evaluates each row against the stored product only: a later row for the same product does not see +/// the mappings or values an earlier row of the same dry run would add, so send one row per product +/// +public class ProductVariantImportDataObject : IRowImport +{ + public const int MaxCombinationsPerProduct = 50; + + private readonly ImportHtmlGuard _htmlGuard; + private readonly IProductAttributeService _productAttributeService; + private readonly IProductService _productService; + private readonly SeoSettings _seoSettings; + + public ProductVariantImportDataObject(IProductService productService, + IProductAttributeService productAttributeService, SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + { + _productService = productService; + _productAttributeService = productAttributeService; + _seoSettings = seoSettings; + _htmlGuard = htmlGuard; + } + + public async Task Import(IReadOnlyList rows, bool dryRun, + CancellationToken cancellationToken = default) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].ProductId, Key(rows[i]))); + continue; + } + + results.Add(await ImportRow(i + 1, rows[i], dryRun)); + } + + return new ImportBatchResult(dryRun, results); + } + + private async Task ImportRow(int row, ProductVariantsDto dto, bool dryRun) + { + var errors = new List(); + var warnings = new List(); + var key = Key(dto); + var attributeDtos = dto.Attributes?.Where(a => a != null).ToList() ?? []; + var combinationDtos = dto.Combinations?.Where(c => c != null).ToList() ?? []; + + var product = await FindProduct(dto, errors); + + if (combinationDtos.Count > MaxCombinationsPerProduct) + errors.Add( + $"At most {MaxCombinationsPerProduct} combinations per product; the row has {combinationDtos.Count}."); + + foreach (var attributeDto in attributeDtos) + { + if (string.IsNullOrEmpty(attributeDto.Attribute)) + errors.Add("Attribute is required."); + if (attributeDto.ControlType.HasValue && !Enum.IsDefined(attributeDto.ControlType.Value)) + errors.Add($"ControlType '{attributeDto.ControlType}' is not valid."); + + foreach (var value in attributeDto.Values ?? []) + { + if (string.IsNullOrEmpty(value?.Name)) + { + errors.Add("Value Name is required."); + continue; + } + + errors.AddRange(_htmlGuard.PlainTextErrors(("Value Name", value.Name), + (nameof(value.ColorSquaresRgb), value.ColorSquaresRgb))); + } + } + + foreach (var combination in combinationDtos) + { + if (combination.Values?.Keys.Any(string.IsNullOrEmpty) == true) + errors.Add("Combination attribute is required."); + errors.AddRange(_htmlGuard.PlainTextErrors((nameof(combination.Sku), combination.Sku), + (nameof(combination.Gtin), combination.Gtin), (nameof(combination.Mpn), combination.Mpn))); + } + + var attributes = await ResolveAttributes(attributeDtos, combinationDtos, errors); + + if (errors.Count > 0) + return new ImportRowResult(row, ImportRowStatus.Rejected, product?.Id ?? "", key, errors, warnings); + + //the product was read from the database, not the cache, so a dry run can change it in memory freely + var changes = new Changes(); + foreach (var attributeDto in attributeDtos) + MergeMapping(product, attributes[attributeDto.Attribute], attributeDto, changes, warnings); + + //the storefront matches a combination only when it names every selectable, combinable mapping + var combinable = product.ProductAttributeMappings.Where(m => IsSelectable(m) && m.Combination).ToList(); + var names = await MappingNames(combinable, attributes); + for (var i = 0; i < combinationDtos.Count; i++) + { + var pairs = MergeCombination(product, i + 1, combinationDtos[i], attributes, changes, warnings); + if (pairs == null) + continue; + foreach (var missing in combinable.Where(m => pairs.All(p => p.Key != m.Id))) + warnings.Add( + $"Combination {i + 1} does not cover '{names[missing.Id]}'; the storefront will not match it."); + } + + if (!dryRun) + await Save(product.Id, changes); + + return new ImportRowResult(row, ImportRowStatus.Updated, product.Id, key, errors, warnings); + } + + private async Task FindProduct(ProductVariantsDto dto, List errors) + { + if (!string.IsNullOrEmpty(dto.ProductId)) + { + var byId = await _productService.GetProductById(dto.ProductId, true); + if (byId == null) + errors.Add($"Product '{dto.ProductId}' was not found."); + return byId; + } + + if (string.IsNullOrEmpty(dto.ProductSku)) + { + errors.Add("ProductId or ProductSku is required."); + return null; + } + + //read from the database, not the cache + var bySku = await _productService.GetProductBySku(dto.ProductSku); + if (bySku == null) + errors.Add($"Product with SKU '{dto.ProductSku}' was not found."); + return bySku; + } + + /// Every attribute key the row uses (mappings and combinations), by id else by normalized SeName + private async Task> ResolveAttributes( + List attributeDtos, List combinationDtos, + List errors) + { + var keys = attributeDtos.Select(a => a.Attribute) + .Concat(combinationDtos.SelectMany(c => c.Values?.Keys ?? [])) + .Where(k => !string.IsNullOrEmpty(k)) + .Distinct(); + + var resolved = new Dictionary(); + IList all = null; + foreach (var attributeKey in keys) + { + var attribute = await _productAttributeService.GetProductAttributeById(attributeKey); + if (attribute == null) + { + //no lookup by SeName exists: load all once and match + all ??= await _productAttributeService.GetAllProductAttributes(); + var seName = GetSeName(attributeKey); + attribute = string.IsNullOrEmpty(seName) + ? null + : all.FirstOrDefault(a => string.Equals(a.SeName, seName, StringComparison.OrdinalIgnoreCase)); + } + + if (attribute == null) + errors.Add($"Attribute '{attributeKey}' was not found."); + else + resolved[attributeKey] = attribute; + } + + return resolved; + } + + /// Attribute name of each mapping, for warnings: from the row's attributes, else from the catalogue + private async Task> MappingNames(List mappings, + Dictionary attributes) + { + var names = new Dictionary(); + foreach (var mapping in mappings) + { + var attribute = attributes.Values.FirstOrDefault(a => a.Id == mapping.ProductAttributeId) ?? + await _productAttributeService.GetProductAttributeById(mapping.ProductAttributeId); + names[mapping.Id] = attribute?.Name ?? mapping.ProductAttributeId; + } + + return names; + } + + private static void MergeMapping(Product product, ProductAttribute attribute, ProductVariantAttributeDto dto, + Changes changes, List warnings) + { + var mapping = product.ProductAttributeMappings.FirstOrDefault(m => m.ProductAttributeId == attribute.Id); + if (mapping == null) + { + mapping = new ProductAttributeMapping { + ProductAttributeId = attribute.Id, + AttributeControlTypeId = dto.ControlType ?? AttributeControlType.DropdownList, + IsRequired = dto.IsRequired ?? true, + Combination = dto.Combination ?? true, + DisplayOrder = dto.DisplayOrder ?? + NextDisplayOrder(product.ProductAttributeMappings.Select(m => m.DisplayOrder)) + }; + product.ProductAttributeMappings.Add(mapping); + changes.NewMappings.Add(mapping); + } + else + { + var wasSelectable = IsSelectable(mapping); + var oldControlType = mapping.AttributeControlTypeId; + //an omitted control type keeps the stored one; only an explicit, different one changes it + var changed = Assign(dto.ControlType, mapping.AttributeControlTypeId, + v => mapping.AttributeControlTypeId = v); + if (changed) + warnings.Add($"Mapping '{attribute.Name}' changed from {oldControlType} to {mapping.AttributeControlTypeId}."); + if (wasSelectable && !IsSelectable(mapping) && + product.ProductAttributeCombinations.Any(c => c.Attributes.Any(a => a.Key == mapping.Id))) + warnings.Add( + $"Mapping '{attribute.Name}' is no longer selectable; its combinations cannot be chosen in the storefront."); + changed |= Assign(dto.IsRequired, mapping.IsRequired, v => mapping.IsRequired = v); + changed |= Assign(dto.Combination, mapping.Combination, v => mapping.Combination = v); + changed |= Assign(dto.DisplayOrder, mapping.DisplayOrder, v => mapping.DisplayOrder = v); + if (changed && !changes.NewMappings.Contains(mapping) && !changes.UpdatedMappings.Contains(mapping)) + changes.UpdatedMappings.Add(mapping); + } + + //a repeated name in the same row matches the value just added, so it collapses into it + foreach (var valueDto in dto.Values ?? []) + { + var value = mapping.ProductAttributeValues + .FirstOrDefault(v => string.Equals(v.Name, valueDto.Name, StringComparison.OrdinalIgnoreCase)); + var isNew = value == null; + if (isNew) + { + value = new ProductAttributeValue { + Name = valueDto.Name, + DisplayOrder = NextDisplayOrder(mapping.ProductAttributeValues.Select(v => v.DisplayOrder)) + }; + mapping.ProductAttributeValues.Add(value); + } + + var changed = Assign(valueDto.PriceAdjustment, value.PriceAdjustment, v => value.PriceAdjustment = v); + changed |= Assign(valueDto.WeightAdjustment, value.WeightAdjustment, v => value.WeightAdjustment = v); + changed |= Assign(valueDto.Cost, value.Cost, v => value.Cost = v); + changed |= Assign(valueDto.IsPreSelected, value.IsPreSelected, v => value.IsPreSelected = v); + changed |= Assign(valueDto.DisplayOrder, value.DisplayOrder, v => value.DisplayOrder = v); + changed |= Assign(valueDto.ColorSquaresRgb, value.ColorSquaresRgb, v => value.ColorSquaresRgb = v); + + //the values of a new mapping are inserted with it + if (changes.NewMappings.Contains(mapping)) + continue; + if (isNew) + changes.NewValues.Add((mapping, value)); + else if (changed && !changes.NewValues.Any(n => n.Value == value) && + !changes.UpdatedValues.Any(u => u.Value == value)) + changes.UpdatedValues.Add((mapping, value)); + } + } + + /// The (mapping, value) pairs of the combination, or null when it was skipped + private static List MergeCombination(Product product, int number, + ProductVariantCombinationDto dto, + Dictionary attributes, Changes changes, List warnings) + { + var prefix = $"Combination {number}"; + if (dto.Values == null || dto.Values.Count == 0) + { + warnings.Add($"{prefix} was skipped: Values are required."); + return null; + } + + if (dto.Price < 0) + { + warnings.Add($"{prefix} was skipped: Price cannot be negative."); + return null; + } + + //stored as the panel stores it: one (mapping id, value id) pair per attribute + var pairs = new List(); + foreach (var (attributeKey, valueName) in dto.Values) + { + var attribute = attributes[attributeKey]; + var mapping = product.ProductAttributeMappings.FirstOrDefault(m => m.ProductAttributeId == attribute.Id); + if (mapping == null) + { + warnings.Add($"{prefix} was skipped: attribute '{attribute.Name}' is not assigned to the product."); + return null; + } + + if (!IsSelectable(mapping)) + { + warnings.Add( + $"{prefix} was skipped: attribute '{attribute.Name}' is a {mapping.AttributeControlTypeId} attribute and has no values to combine."); + return null; + } + + //the storefront ignores such a mapping when it looks a combination up, so it would never be found + if (!mapping.Combination) + { + warnings.Add( + $"{prefix} was skipped: attribute '{attribute.Name}' does not take part in combinations (Combination is off)."); + return null; + } + + if (pairs.Any(p => p.Key == mapping.Id)) + { + warnings.Add($"{prefix} was skipped: attribute '{attribute.Name}' is named more than once."); + return null; + } + + var value = string.IsNullOrEmpty(valueName) + ? null + : mapping.ProductAttributeValues.FirstOrDefault(v => + string.Equals(v.Name, valueName, StringComparison.OrdinalIgnoreCase)); + if (value == null) + { + warnings.Add($"{prefix} was skipped: attribute '{attribute.Name}' has no value '{valueName}'."); + return null; + } + + pairs.Add(new CustomAttribute { Key = mapping.Id, Value = value.Id }); + } + + var combination = product.ProductAttributeCombinations.FirstOrDefault(c => SamePairs(c.Attributes, pairs)); + + if (!string.IsNullOrEmpty(dto.Sku) && product.ProductAttributeCombinations.Any(c => + c != combination && string.Equals(c.Sku, dto.Sku, StringComparison.OrdinalIgnoreCase))) + { + warnings.Add( + $"{prefix} was skipped: SKU '{dto.Sku}' is already used by another combination of this product."); + return null; + } + + if (combination == null) + { + //same defaults as the panel's "generate all combinations" + combination = new ProductAttributeCombination { + Attributes = pairs, + Sku = dto.Sku, + Gtin = dto.Gtin, + Mpn = dto.Mpn, + OverriddenPrice = dto.Price, + StockQuantity = 0, + AllowOutOfStockOrders = false, + NotifyAdminForQuantityBelow = 1 + }; + product.ProductAttributeCombinations.Add(combination); + changes.NewCombinations.Add(combination); + return pairs; + } + + var changed = Assign(dto.Sku, combination.Sku, v => combination.Sku = v); + changed |= Assign(dto.Gtin, combination.Gtin, v => combination.Gtin = v); + changed |= Assign(dto.Mpn, combination.Mpn, v => combination.Mpn = v); + if (dto.Price.HasValue && combination.OverriddenPrice != dto.Price) + { + combination.OverriddenPrice = dto.Price; + changed = true; + } + + if (changed && !changes.NewCombinations.Contains(combination) && + !changes.UpdatedCombinations.Contains(combination)) + changes.UpdatedCombinations.Add(combination); + + return pairs; + } + + private async Task Save(string productId, Changes changes) + { + foreach (var mapping in changes.NewMappings) + await _productAttributeService.InsertProductAttributeMapping(mapping, productId); + //an update writes the whole mapping, its values included + foreach (var mapping in changes.UpdatedMappings) + await _productAttributeService.UpdateProductAttributeMapping(mapping, productId); + foreach (var (mapping, value) in changes.NewValues.Where(v => !changes.UpdatedMappings.Contains(v.Mapping))) + await _productAttributeService.InsertProductAttributeValue(value, productId, mapping.Id); + foreach (var (mapping, value) in changes.UpdatedValues.Where(v => !changes.UpdatedMappings.Contains(v.Mapping))) + await _productAttributeService.UpdateProductAttributeValue(value, productId, mapping.Id); + foreach (var combination in changes.NewCombinations) + await _productAttributeService.InsertProductAttributeCombination(combination, productId); + foreach (var combination in changes.UpdatedCombinations) + await _productAttributeService.UpdateProductAttributeCombination(combination, productId); + } + + /// TextBox, MultilineTextbox, Datepicker, FileUpload and Hidden have no values a shopper selects + private static bool IsSelectable(ProductAttributeMapping mapping) + { + return mapping.ShouldHaveValues() && mapping.AttributeControlTypeId != AttributeControlType.Hidden; + } + + private static bool SamePairs(IList stored, List pairs) + { + if (stored == null || stored.Count != pairs.Count) + return false; + + var set = stored.Select(a => (a.Key, a.Value)).ToHashSet(); + return set.Count == pairs.Count && pairs.All(p => set.Contains((p.Key, p.Value))); + } + + private static int NextDisplayOrder(IEnumerable orders) + { + var list = orders.ToList(); + return list.Count == 0 ? 0 : list.Max() + 1; + } + + private static bool Assign(T? provided, T current, Action set) where T : struct + { + if (!provided.HasValue || EqualityComparer.Default.Equals(provided.Value, current)) + return false; + + set(provided.Value); + return true; + } + + private static bool Assign(string provided, string current, Action set) + { + if (provided == null || provided == current) + return false; + + set(provided); + return true; + } + + private static string Key(ProductVariantsDto dto) + { + return !string.IsNullOrEmpty(dto.ProductSku) ? dto.ProductSku : dto.ProductId ?? ""; + } + + //same normalization as the product attribute import + private string GetSeName(string name) + { + return SeoExtensions.GetSeName(name, _seoSettings.ConvertNonWesternChars, _seoSettings.AllowUnicodeCharsInUrls, + _seoSettings.SeoCharConversion); + } + + private sealed class Changes + { + public List NewMappings { get; } = []; + public List UpdatedMappings { get; } = []; + public List<(ProductAttributeMapping Mapping, ProductAttributeValue Value)> NewValues { get; } = []; + public List<(ProductAttributeMapping Mapping, ProductAttributeValue Value)> UpdatedValues { get; } = []; + public List NewCombinations { get; } = []; + public List UpdatedCombinations { get; } = []; + } +} diff --git a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs index ed93673ecb..d664c2f8ea 100644 --- a/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Catalog/Startup/StartupApplication.cs @@ -111,6 +111,7 @@ private static void RegisterExportImport(IServiceCollection serviceCollection) serviceCollection.AddScoped(); serviceCollection.AddScoped, SpecificationAttributeImportDataObject>(); serviceCollection.AddScoped, ProductAttributeImportDataObject>(); + serviceCollection.AddScoped, ProductVariantImportDataObject>(); serviceCollection.AddScoped(); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); serviceCollection.AddScoped>(sp => sp.GetRequiredService()); diff --git a/src/Business/Grand.Business.Core/Dto/ProductVariantsDto.cs b/src/Business/Grand.Business.Core/Dto/ProductVariantsDto.cs new file mode 100644 index 0000000000..1361930284 --- /dev/null +++ b/src/Business/Grand.Business.Core/Dto/ProductVariantsDto.cs @@ -0,0 +1,57 @@ +using Grand.Domain.Catalog; + +namespace Grand.Business.Core.Dto; + +/// +/// The variants of one existing product: attribute mappings with their values, and an explicit list of +/// combinations. Merged into what is stored; nothing is deleted +/// +public class ProductVariantsDto +{ + /// Product to change; when set and not found the row is rejected + public string ProductId { get; set; } + + /// Used to find the product when ProductId is empty + public string ProductSku { get; set; } + + public IList Attributes { get; set; } + public IList Combinations { get; set; } +} + +public class ProductVariantAttributeDto +{ + /// Catalogue product attribute by id or SeName; it must exist + public string Attribute { get; set; } + + public AttributeControlType? ControlType { get; set; } + public bool? IsRequired { get; set; } + public bool? Combination { get; set; } + public int? DisplayOrder { get; set; } + + /// Values of the mapping, merged by name (case-insensitive): missing values are added, provided fields updated + public IList Values { get; set; } +} + +public class ProductVariantValueDto +{ + public string Name { get; set; } + public double? PriceAdjustment { get; set; } + public double? WeightAdjustment { get; set; } + public double? Cost { get; set; } + public bool? IsPreSelected { get; set; } + public string ColorSquaresRgb { get; set; } + public int? DisplayOrder { get; set; } +} + +public class ProductVariantCombinationDto +{ + /// Attribute (id or SeName) to value name; the set identifies the combination, order does not matter + public IDictionary Values { get; set; } + + public string Sku { get; set; } + public string Gtin { get; set; } + public string Mpn { get; set; } + + /// Overridden price of the combination + public double? Price { get; set; } +} diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs new file mode 100644 index 0000000000..ea0d04ba7b --- /dev/null +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs @@ -0,0 +1,618 @@ +using Grand.Business.Catalog.Services.ExportImport; +using Grand.Business.Catalog.Services.Products; +using Grand.Business.Core.Dto; +using Grand.Business.Core.Interfaces.Catalog.Products; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Data; +using Grand.Data.Tests.MongoDb; +using Grand.Domain.Catalog; +using Grand.Domain.Common; +using Grand.Domain.Seo; +using Grand.Infrastructure.Caching; +using Grand.Infrastructure.Configuration; +using Grand.Infrastructure.Security; +using Grand.Infrastructure.Tests.Caching; +using Grand.Mediator; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Catalog.Tests.Services.ExportImport; + +/// Runs against the real ProductAttributeService over the test Mongo database +[TestClass] +public class ProductVariantImportDataObjectTests +{ + private Mock _attributeService; + private ProductAttribute _color; + private ProductVariantImportDataObject _import; + private Product _product; + private IRepository _productRepository; + private Mock _productService; + private ProductAttribute _size; + + [TestInitialize] + public async Task Init() + { + _productRepository = new MongoDBRepositoryTest(); + var attributeRepository = new MongoDBRepositoryTest(); + var mediator = new Mock(); + var cache = new MemoryCacheBase(MemoryCacheTest.Get(), mediator.Object, + new CacheConfig { DefaultCacheTimeMinutes = 1 }); + _attributeService = + new Mock(cache, attributeRepository, _productRepository, mediator.Object) { + CallBase = true + }; + + _color = new ProductAttribute { Name = "Color", SeName = "color" }; + _size = new ProductAttribute { Name = "Shirt Size", SeName = "shirt-size" }; + await attributeRepository.InsertAsync(_color); + await attributeRepository.InsertAsync(_size); + + _product = new Product { Name = "Shirt", Sku = "SHIRT" }; + await _productRepository.InsertAsync(_product); + + _productService = new Mock(); + _productService.Setup(s => s.GetProductById(It.IsAny(), It.IsAny())) + .Returns((string id, bool _) => _productRepository.GetByIdAsync(id)); + _productService.Setup(s => s.GetProductBySku(It.IsAny())) + .Returns((string sku) => _productRepository.GetOneAsync(p => p.Sku == sku)); + + var securityConfig = new SecurityConfig(); + var guard = new ImportHtmlGuard(new HtmlSanitizationService(securityConfig), securityConfig); + _import = new ProductVariantImportDataObject(_productService.Object, _attributeService.Object, + new SeoSettings(), guard); + } + + private int Writes => _attributeService.Invocations.Count(i => + i.Method.Name.StartsWith("Insert") || i.Method.Name.StartsWith("Update") || i.Method.Name.StartsWith("Delete")); + + private Task Stored() + { + return _productRepository.GetByIdAsync(_product.Id); + } + + private static ProductVariantsDto ShirtRow(params ProductVariantCombinationDto[] combinations) + { + return new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [ + new ProductVariantAttributeDto { + Attribute = "color", + Values = [new ProductVariantValueDto { Name = "Red", PriceAdjustment = 1 }, new ProductVariantValueDto { Name = "Blue" }] + }, + new ProductVariantAttributeDto { + Attribute = "Shirt Size", + Values = [new ProductVariantValueDto { Name = "M" }, new ProductVariantValueDto { Name = "XL" }] + } + ], + Combinations = combinations.ToList() + }; + } + + private static ProductVariantCombinationDto Combination(string color, string size, string sku = null, + double? price = null) + { + return new ProductVariantCombinationDto { + Values = new Dictionary { ["color"] = color, ["shirt-size"] = size }, + Sku = sku, Price = price + }; + } + + private static string ValueId(Product product, ProductAttribute attribute, string name) + { + return product.ProductAttributeMappings.Single(m => m.ProductAttributeId == attribute.Id) + .ProductAttributeValues.Single(v => v.Name == name).Id; + } + + private static string MappingId(Product product, ProductAttribute attribute) + { + return product.ProductAttributeMappings.Single(m => m.ProductAttributeId == attribute.Id).Id; + } + + [TestMethod] + public async Task Create_MappingsValuesAndCombination() + { + var result = await _import.Import([ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL", 12))], false); + + var row = result.Rows.Single(); + Assert.AreEqual(ImportRowStatus.Updated, row.Status, string.Join(" ", row.Errors)); + Assert.AreEqual(_product.Id, row.Id); + Assert.AreEqual("SHIRT", row.Key); + + var stored = await Stored(); + Assert.HasCount(2, stored.ProductAttributeMappings); + var color = stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id); + Assert.AreEqual(AttributeControlType.DropdownList, color.AttributeControlTypeId); + Assert.IsTrue(color.IsRequired); + Assert.IsTrue(color.Combination); + Assert.AreEqual(0, color.DisplayOrder); + Assert.AreEqual(1, stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _size.Id).DisplayOrder); + Assert.AreEqual(1, color.ProductAttributeValues.Single(v => v.Name == "Red").PriceAdjustment); + Assert.HasCount(2, color.ProductAttributeValues); + + var combination = stored.ProductAttributeCombinations.Single(); + Assert.AreEqual("SHIRT-RED-XL", combination.Sku); + Assert.AreEqual(12, combination.OverriddenPrice); + Assert.HasCount(2, combination.Attributes); + Assert.IsTrue(combination.Attributes.Any(a => + a.Key == MappingId(stored, _color) && a.Value == ValueId(stored, _color, "Red"))); + Assert.IsTrue(combination.Attributes.Any(a => + a.Key == MappingId(stored, _size) && a.Value == ValueId(stored, _size, "XL"))); + } + + [TestMethod] + public async Task Resend_IsNoOp_NoDuplicates() + { + var row = ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL"), Combination("Blue", "M", "SHIRT-BLUE-M")); + await _import.Import([row], false); + + var again = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Updated, again.Rows.Single().Status); + Assert.IsEmpty(again.Rows.Single().Warnings); + var stored = await Stored(); + Assert.HasCount(2, stored.ProductAttributeMappings); + Assert.IsTrue(stored.ProductAttributeMappings.All(m => m.ProductAttributeValues.Count == 2)); + Assert.HasCount(2, stored.ProductAttributeCombinations); + } + + [TestMethod] + public async Task Resend_WritesNothing() + { + var row = ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL")); + await _import.Import([row], false); + var writes = Writes; + + await _import.Import([row], false); + + Assert.AreEqual(writes, Writes); + } + + [TestMethod] + public async Task Combination_MatchIsOrderIndependent() + { + await _import.Import([ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL"))], false); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Combinations = [ + new ProductVariantCombinationDto { + Values = new Dictionary { ["Shirt Size"] = "xl", ["Color"] = "RED" }, Price = 15 + } + ] + } + ], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + var combination = (await Stored()).ProductAttributeCombinations.Single(); + Assert.AreEqual(15, combination.OverriddenPrice); + } + + [TestMethod] + public async Task Combination_PartialUpdate_KeepsSkuGtinMpn() + { + var first = ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL", 12)); + first.Combinations[0].Gtin = "123"; + first.Combinations[0].Mpn = "MPN-1"; + await _import.Import([first], false); + + await _import.Import([ + new ProductVariantsDto { ProductSku = "SHIRT", Combinations = [Combination("Red", "XL", price: 20)] } + ], false); + + var combination = (await Stored()).ProductAttributeCombinations.Single(); + Assert.AreEqual(20, combination.OverriddenPrice); + Assert.AreEqual("SHIRT-RED-XL", combination.Sku); + Assert.AreEqual("123", combination.Gtin); + Assert.AreEqual("MPN-1", combination.Mpn); + } + + [TestMethod] + public async Task UnknownProduct_Rejected() + { + var result = await _import.Import([ + new ProductVariantsDto { ProductId = "nope", ProductSku = "SHIRT" }, + new ProductVariantsDto { ProductSku = "NO-SUCH-SKU" }, + new ProductVariantsDto() + ], false); + + Assert.AreEqual(3, result.Rejected); + Assert.AreEqual("Product 'nope' was not found.", result.Rows[0].Errors.Single()); + Assert.AreEqual("Product with SKU 'NO-SUCH-SKU' was not found.", result.Rows[1].Errors.Single()); + Assert.AreEqual("ProductId or ProductSku is required.", result.Rows[2].Errors.Single()); + Assert.AreEqual(0, Writes); + } + + [TestMethod] + public async Task UnknownAttribute_RejectsWholeRow() + { + var row = ShirtRow(Combination("Red", "XL")); + row.Attributes.Add(new ProductVariantAttributeDto { Attribute = "material", Values = [new() { Name = "Wool" }] }); + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows.Single().Status); + Assert.Contains("Attribute 'material' was not found.", result.Rows.Single().Errors); + Assert.AreEqual(0, Writes); + Assert.IsEmpty((await Stored()).ProductAttributeMappings); + } + + [TestMethod] + public async Task UnknownAttributeInCombination_RejectsWholeRow() + { + var row = ShirtRow(new ProductVariantCombinationDto { + Values = new Dictionary { ["color"] = "Red", ["material"] = "Wool" } + }); + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows.Single().Status); + Assert.Contains("Attribute 'material' was not found.", result.Rows.Single().Errors); + Assert.AreEqual(0, Writes); + } + + [TestMethod] + public async Task AttributeById_IsMatched() + { + var result = await _import.Import([ + new ProductVariantsDto { + ProductId = _product.Id, + Attributes = [new ProductVariantAttributeDto { Attribute = _color.Id, Values = [new() { Name = "Red" }] }] + } + ], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + Assert.AreEqual(_product.Id, result.Rows.Single().Key); + Assert.AreEqual(_color.Id, (await Stored()).ProductAttributeMappings.Single().ProductAttributeId); + } + + [TestMethod] + public async Task TextBoxAttributeInCombination_CombinationSkippedWithWarning() + { + var row = ShirtRow(Combination("Red", "XL", "OK"), + new ProductVariantCombinationDto { + Values = new Dictionary { ["color"] = "Blue", ["shirt-size"] = "M" }, Sku = "TB" + }); + row.Attributes[1].ControlType = AttributeControlType.TextBox; + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + Assert.IsTrue(result.Rows.Single().Warnings.Any(w => w.Contains("Shirt Size") && w.Contains("TextBox")), + string.Join(" ", result.Rows.Single().Warnings)); + Assert.IsEmpty((await Stored()).ProductAttributeCombinations); + } + + [TestMethod] + public async Task CombinationNotCoveringEveryMapping_StoredWithWarning() + { + var result = await _import.Import([ + ShirtRow(new ProductVariantCombinationDto { + Values = new Dictionary { ["color"] = "Red" }, Sku = "RED-ONLY" + }) + ], false); + + var row = result.Rows.Single(); + Assert.AreEqual(ImportRowStatus.Updated, row.Status); + Assert.AreEqual("Combination 1 does not cover 'Shirt Size'; the storefront will not match it.", + row.Warnings.Single()); + Assert.AreEqual("RED-ONLY", (await Stored()).ProductAttributeCombinations.Single().Sku); + } + + [TestMethod] + public async Task EmptyAttributeKeyInCombination_RejectsRow() + { + var result = await _import.Import([ + ShirtRow(new ProductVariantCombinationDto { + Values = new Dictionary { [""] = "Red", ["shirt-size"] = "XL" } + }) + ], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows.Single().Status); + Assert.Contains("Combination attribute is required.", result.Rows.Single().Errors); + Assert.AreEqual(0, Writes); + } + + [TestMethod] + public async Task CombinationOffAttributeInCombination_SkippedWithWarning() + { + var row = ShirtRow(Combination("Red", "XL", "X")); + row.Attributes[1].Combination = false; + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + Assert.AreEqual( + "Combination 1 was skipped: attribute 'Shirt Size' does not take part in combinations (Combination is off).", + result.Rows.Single().Warnings.Single()); + Assert.IsEmpty((await Stored()).ProductAttributeCombinations); + } + + [TestMethod] + public async Task HiddenAttributeInCombination_SkippedWithWarning() + { + var row = ShirtRow(Combination("Red", "XL", "X")); + row.Attributes[1].ControlType = AttributeControlType.Hidden; + + var result = await _import.Import([row], false); + + Assert.AreEqual( + "Combination 1 was skipped: attribute 'Shirt Size' is a Hidden attribute and has no values to combine.", + result.Rows.Single().Warnings.Single()); + Assert.IsEmpty((await Stored()).ProductAttributeCombinations); + } + + [TestMethod] + public async Task MappingTurnedNonSelectable_WithCombinations_Warns() + { + await _import.Import([ShirtRow(Combination("Red", "XL", "X"))], false); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", ControlType = AttributeControlType.TextBox }] + } + ], false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + CollectionAssert.AreEquivalent(new[] { + "Mapping 'Color' changed from DropdownList to TextBox.", + "Mapping 'Color' is no longer selectable; its combinations cannot be chosen in the storefront." + }, result.Rows.Single().Warnings.ToArray()); + Assert.AreEqual(AttributeControlType.TextBox, + (await Stored()).ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id).AttributeControlTypeId); + } + + [TestMethod] + public async Task DuplicateCombinationSku_SkippedWithWarning() + { + await _import.Import([ShirtRow(Combination("Red", "XL", "DUP"))], false); + + var result = await _import.Import([ + ShirtRow(Combination("Blue", "M", "dup"), Combination("Red", "M", "NEW"), Combination("Blue", "XL", "NEW")) + ], false); + + var row = result.Rows.Single(); + Assert.AreEqual(ImportRowStatus.Updated, row.Status); + Assert.HasCount(2, row.Warnings, string.Join(" ", row.Warnings)); + Assert.IsTrue(row.Warnings.All(w => w.Contains("SKU"))); + var stored = await Stored(); + Assert.HasCount(2, stored.ProductAttributeCombinations); + Assert.AreEqual(1, stored.ProductAttributeCombinations.Count(c => c.Sku == "NEW")); + } + + [TestMethod] + public async Task MoreThan50Combinations_Rejected() + { + var row = ShirtRow(Enumerable.Range(0, 51).Select(i => Combination("Red", "XL", $"S{i}")).ToArray()); + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows.Single().Status); + Assert.AreEqual(0, Writes); + } + + [TestMethod] + public async Task ValueUpdate_ChangesOnlyProvidedFields() + { + var create = ShirtRow(); + create.Attributes[0].Values[0] = new ProductVariantValueDto { + Name = "Red", PriceAdjustment = 1, WeightAdjustment = 2, Cost = 3, IsPreSelected = true, + ColorSquaresRgb = "#ff0000", DisplayOrder = 4 + }; + await _import.Import([create], false); + + await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", Values = [new() { Name = "RED", Cost = 9 }] }] + } + ], false); + + var stored = await Stored(); + var color = stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id); + var red = color.ProductAttributeValues.Single(v => v.Name == "Red"); + Assert.AreEqual(9, red.Cost); + Assert.AreEqual(1, red.PriceAdjustment); + Assert.AreEqual(2, red.WeightAdjustment); + Assert.IsTrue(red.IsPreSelected); + Assert.AreEqual("#ff0000", red.ColorSquaresRgb); + Assert.AreEqual(4, red.DisplayOrder); + Assert.HasCount(2, color.ProductAttributeValues); + } + + private async Task StoreColorSquaresMapping() + { + await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [ + new ProductVariantAttributeDto { + Attribute = "color", ControlType = AttributeControlType.ColorSquares, IsRequired = false, + Combination = false, Values = [new() { Name = "Red", ColorSquaresRgb = "#ff0000" }] + } + ] + } + ], false); + } + + [TestMethod] + public async Task MappingUpdate_OmittedControlTypeRequiredAndCombination_AreKept() + { + await StoreColorSquaresMapping(); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", Values = [new() { Name = "Blue" }] }] + } + ], false); + + Assert.IsEmpty(result.Rows.Single().Warnings); + var color = (await Stored()).ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id); + Assert.AreEqual(AttributeControlType.ColorSquares, color.AttributeControlTypeId); + Assert.IsFalse(color.IsRequired); + Assert.IsFalse(color.Combination); + CollectionAssert.AreEquivalent(new[] { "Red", "Blue" }, color.ProductAttributeValues.Select(v => v.Name).ToArray()); + } + + [TestMethod] + public async Task MappingUpdate_ExplicitControlTypeChange_Warns() + { + await StoreColorSquaresMapping(); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", ControlType = AttributeControlType.RadioList }] + } + ], false); + + Assert.AreEqual("Mapping 'Color' changed from ColorSquares to RadioList.", result.Rows.Single().Warnings.Single()); + Assert.AreEqual(AttributeControlType.RadioList, + (await Stored()).ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id).AttributeControlTypeId); + } + + [TestMethod] + public async Task MappingUpdate_ExplicitSameControlType_DoesNotWarn() + { + await StoreColorSquaresMapping(); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", ControlType = AttributeControlType.ColorSquares }] + } + ], false); + + Assert.IsEmpty(result.Rows.Single().Warnings); + } + + [TestMethod] + public async Task MappingUpdate_ChangesOnlyProvidedFields_KeepsValues() + { + await _import.Import([ShirtRow()], false); + + await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", IsRequired = false, Values = [new() { Name = "Green" }] }] + } + ], false); + + var color = (await Stored()).ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id); + Assert.IsFalse(color.IsRequired); + Assert.AreEqual(AttributeControlType.DropdownList, color.AttributeControlTypeId); + Assert.IsTrue(color.Combination); + CollectionAssert.AreEquivalent(new[] { "Red", "Blue", "Green" }, + color.ProductAttributeValues.Select(v => v.Name).ToArray()); + } + + [TestMethod] + public async Task NewValueOnExistingMapping_IsInsertedAndCombinable() + { + await _import.Import([ShirtRow()], false); + + var result = await _import.Import([ + new ProductVariantsDto { + ProductSku = "SHIRT", + Attributes = [new ProductVariantAttributeDto { Attribute = "color", Values = [new() { Name = "Green" }] }], + Combinations = [Combination("Green", "M", "SHIRT-GREEN-M")] + } + ], false); + + Assert.IsEmpty(result.Rows.Single().Warnings); + Assert.AreEqual(1, _attributeService.Invocations.Count(i => i.Method.Name == "InsertProductAttributeValue")); + var stored = await Stored(); + Assert.HasCount(3, stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id) + .ProductAttributeValues); + Assert.IsTrue(stored.ProductAttributeCombinations.Single().Attributes + .Any(a => a.Value == ValueId(stored, _color, "Green"))); + } + + [TestMethod] + public async Task DryRun_WritesNothing() + { + var result = await _import.Import([ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL"))], true); + + Assert.IsTrue(result.DryRun); + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + Assert.AreEqual(0, Writes); + var stored = await Stored(); + Assert.IsEmpty(stored.ProductAttributeMappings); + Assert.IsEmpty(stored.ProductAttributeCombinations); + } + + [TestMethod] + public async Task DryRun_OnExistingVariants_WritesNothing() + { + await _import.Import([ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL"))], false); + var writes = Writes; + var row = ShirtRow(Combination("Red", "XL", "CHANGED", 99), Combination("Blue", "M", "NEW")); + row.Attributes[0].Values.Add(new ProductVariantValueDto { Name = "Green" }); + row.Attributes[0].IsRequired = false; + + var result = await _import.Import([row], true); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows.Single().Status); + Assert.AreEqual(writes, Writes); + var stored = await Stored(); + Assert.HasCount(1, stored.ProductAttributeCombinations); + Assert.AreEqual("SHIRT-RED-XL", stored.ProductAttributeCombinations.Single().Sku); + Assert.HasCount(2, stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id) + .ProductAttributeValues); + } + + [TestMethod] + public async Task CancelledBeforeSecondRow_SecondRowNotProcessed() + { + using var cts = new CancellationTokenSource(); + _attributeService.Setup(s => s.InsertProductAttributeMapping(It.IsAny(), It.IsAny())) + .Callback(() => cts.Cancel()).CallBase(); + + var result = await _import.Import([ + ShirtRow(), + new ProductVariantsDto { ProductSku = "SHIRT", Combinations = [Combination("Red", "XL", "X")] } + ], false, cts.Token); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[1].Status); + Assert.AreEqual(ImportRowResult.NotProcessedError, result.Rows[1].Errors.Single()); + Assert.AreEqual("SHIRT", result.Rows[1].Key); + Assert.IsEmpty((await Stored()).ProductAttributeCombinations); + } + + [TestMethod] + public async Task MarkupInValueName_Rejected() + { + var row = ShirtRow(); + row.Attributes[0].Values.Add(new ProductVariantValueDto { Name = "" }); + + var result = await _import.Import([row], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows.Single().Status); + Assert.AreEqual(0, Writes); + } + + [TestMethod] + public async Task StorefrontLookup_FindsCreatedCombination() + { + await _import.Import([ShirtRow(Combination("Red", "XL", "SHIRT-RED-XL", 12))], false); + var stored = await Stored(); + + //the storefront builds the selection mapping by mapping, in display order, as the attribute parser does + IList selection = []; + selection = ProductExtensions.AddProductAttribute(selection, + stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _size.Id), ValueId(stored, _size, "XL")); + selection = ProductExtensions.AddProductAttribute(selection, + stored.ProductAttributeMappings.Single(m => m.ProductAttributeId == _color.Id), ValueId(stored, _color, "Red")); + + var found = stored.FindProductAttributeCombination(selection); + + Assert.IsNotNull(found); + Assert.AreEqual("SHIRT-RED-XL", found.Sku); + Assert.AreEqual("SHIRT-RED-XL", stored.FormatSku(selection)); + Assert.HasCount(2, stored.ParseProductAttributeValues(found.Attributes)); + } +} From 5763990bce5e4bc85d4f93588bdd21180b4a1054 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sat, 10 Oct 2026 08:21:51 +0200 Subject: [PATCH 5/6] Catalog import: review fixes - name matching, unique SeName/Sku, per-row failure isolation - Name-only rows match by the stored name, not by the slug the name would get, so an entity whose slug was renumbered (apple-2) is found; a name shared by several entities is rejected - Specification and product attributes: a SeName already used by another attribute is rejected - Products (row API): a Sku already used by another product is rejected - A row that throws is reported rejected (FailedError) and the batch goes on; the panel's spreadsheet import still throws - Row-created products get the remaining panel defaults (MaxEnteredPrice, downloads, recurring) - Brand, categories, collections and parent category are read once per row; product attributes once per batch - Shared batch loop, ImportMode and slug lookup moved to ImportRows; dead LowStock line removed --- .../ExportImport/BrandImportDataObject.cs | 65 +++-------- .../ExportImport/CategoryImportDataObject.cs | 90 ++++++--------- .../CollectionImportDataObject.cs | 68 ++++------- .../Services/ExportImport/ImportRows.cs | 79 +++++++++++-- .../ProductAttributeImportDataObject.cs | 66 ++++++----- .../ExportImport/ProductImportDataObject.cs | 108 +++++++++--------- .../ProductVariantImportDataObject.cs | 29 +++-- .../SpecificationAttributeImportDataObject.cs | 33 +++--- .../Interfaces/ExportImport/IRowImport.cs | 13 ++- .../AttributeImportDataObjectTests.cs | 73 +++++++++++- .../BrandImportDataObjectTests.cs | 33 +++++- .../CategoryImportDataObjectTests.cs | 22 +++- .../CollectionImportDataObjectTests.cs | 2 +- .../ProductImportDataObjectTests.cs | 90 +++++++++++++++ .../ProductVariantImportDataObjectTests.cs | 3 +- 15 files changed, 483 insertions(+), 291 deletions(-) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs index a8f88eff8f..934ad5569c 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs @@ -22,7 +22,6 @@ public class BrandImportDataObject : IImportDataObject, IRowImport _logger; public BrandImportDataObject( @@ -33,7 +32,6 @@ public BrandImportDataObject( ISeNameService seNameService, SecurityConfig securityConfig, ImportHtmlGuard htmlGuard, - SeoSettings seoSettings, ILogger logger) { _brandService = brandService; @@ -43,7 +41,6 @@ public BrandImportDataObject( _seNameService = seNameService; _securityConfig = securityConfig; _htmlGuard = htmlGuard; - _seoSettings = seoSettings; _logger = logger; } @@ -60,32 +57,12 @@ public Task Import(IReadOnlyList rows, bool dryRun, return Import(rows, dryRun, ImportMode.Row, cancellationToken); } - /// - /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. - /// - private enum ImportMode - { - Row, - Panel - } - - private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + /// Row: unknown Id rejected, SeName/Name matching. Panel (XLSX): unknown Id creates the entity with that Id, no matching. + private Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, (row, dto) => ImportRow(row, dto, dryRun, mode), + mode == ImportMode.Row, _logger, cancellationToken); } private async Task ImportRow(int row, BrandDto dto, bool dryRun, ImportMode mode) @@ -94,7 +71,9 @@ private async Task ImportRow(int row, BrandDto dto, bool dryRun var warnings = new List(); var key = Key(dto); - var (existing, matchedByName) = await FindBrand(dto, mode); + var (existing, matchedByName, matchError) = await FindBrand(dto, mode); + if (matchError != null) + errors.Add(matchError); if (matchedByName) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) @@ -144,35 +123,27 @@ private static string Key(BrandDto dto) } /// - /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug - /// its Name would get - accepted only when the stored name is the same (any case) + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by its stored + /// name (any case) - more than one brand of that name is an error /// - private async Task<(Brand brand, bool matchedByName)> FindBrand(BrandDto dto, ImportMode mode) + private async Task<(Brand brand, bool matchedByName, string error)> FindBrand(BrandDto dto, ImportMode mode) { if (!string.IsNullOrEmpty(dto.Id)) - return (await _brandService.GetBrandById(dto.Id), false); + return (await _brandService.GetBrandById(dto.Id), false, null); if (mode == ImportMode.Panel) - return (null, false); + return (null, false, null); if (!string.IsNullOrEmpty(dto.SeName)) - return (await FindBrandBySlug(dto.SeName), false); + return (await ImportRows.FindBySlug(_slugService, dto.SeName, EntityTypes.Brand, + _brandService.GetBrandById), false, null); if (string.IsNullOrEmpty(dto.Name)) - return (null, false); - - var found = await FindBrandBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); - var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase); - return matches ? (found, true) : (null, false); - } - - private async Task FindBrandBySlug(string slug) - { - var url = await _slugService.GetBySlug(slug); - if (url is not { EntityName: EntityTypes.Brand } || string.IsNullOrEmpty(url.EntityId)) - return null; + return (null, false, null); - return await _brandService.GetBrandById(url.EntityId); + var candidates = await _brandService.GetAllBrands(dto.Name, "", showHidden: true); + var (found, error) = ImportRows.SingleByName(candidates, b => b.Name, dto.Name, "brand"); + return (found, found != null, error); } private async Task UpdateBrandData(BrandDto brandDto, Brand brand) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs index e92402833b..21a7971242 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs @@ -22,7 +22,6 @@ public class CategoryImportDataObject : IImportDataObject, IRowImpo private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; private readonly ImportHtmlGuard _htmlGuard; - private readonly SeoSettings _seoSettings; private readonly ILogger _logger; public CategoryImportDataObject( @@ -33,7 +32,6 @@ public CategoryImportDataObject( ISeNameService seNameService, SecurityConfig securityConfig, ImportHtmlGuard htmlGuard, - SeoSettings seoSettings, ILogger logger) { _categoryService = categoryService; @@ -43,7 +41,6 @@ public CategoryImportDataObject( _seNameService = seNameService; _securityConfig = securityConfig; _htmlGuard = htmlGuard; - _seoSettings = seoSettings; _logger = logger; } @@ -60,32 +57,12 @@ public Task Import(IReadOnlyList rows, bool dryR return Import(rows, dryRun, ImportMode.Row, cancellationToken); } - /// - /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. - /// - private enum ImportMode - { - Row, - Panel - } - - private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + /// Row: unknown Id rejected, SeName/Name matching. Panel (XLSX): unknown Id creates the entity with that Id, no matching. + private Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, (row, dto) => ImportRow(row, dto, dryRun, mode), + mode == ImportMode.Row, _logger, cancellationToken); } private async Task ImportRow(int row, CategoryDto dto, bool dryRun, ImportMode mode) @@ -94,7 +71,9 @@ private async Task ImportRow(int row, CategoryDto dto, bool dry var warnings = new List(); var key = Key(dto); - var (existing, matchedByName) = await FindCategory(dto, mode); + var (existing, matchedByName, matchError) = await FindCategory(dto, mode); + if (matchError != null) + errors.Add(matchError); if (matchedByName) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) @@ -110,7 +89,9 @@ private async Task ImportRow(int row, CategoryDto dto, bool dry (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), (nameof(dto.MetaTitle), dto.MetaTitle))); - if (!string.IsNullOrEmpty(dto.ParentCategoryId) && await _categoryService.GetCategoryById(dto.ParentCategoryId) == null) + var parentMissing = !string.IsNullOrEmpty(dto.ParentCategoryId) && + await _categoryService.GetCategoryById(dto.ParentCategoryId) == null; + if (parentMissing) warnings.Add($"ParentCategoryId '{dto.ParentCategoryId}' was not found and was cleared."); if (errors.Count > 0) @@ -136,7 +117,7 @@ private async Task ImportRow(int row, CategoryDto dto, bool dry if (isNew) await _categoryService.InsertCategory(category); else await _categoryService.UpdateCategory(category); - await UpdateCategoryData(dto, category); + await UpdateCategoryData(dto, category, parentMissing); return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, category.Id, key, errors, warnings); } @@ -147,40 +128,32 @@ private static string Key(CategoryDto dto) } /// - /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug - /// its Name would get - accepted only when the stored name is the same (any case) and, when the row names a - /// parent, under that parent + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by its stored + /// name (any case) - under the row's parent when it names one; more than one category of that name is an error /// - private async Task<(Category category, bool matchedByName)> FindCategory(CategoryDto dto, ImportMode mode) + private async Task<(Category category, bool matchedByName, string error)> FindCategory(CategoryDto dto, + ImportMode mode) { if (!string.IsNullOrEmpty(dto.Id)) - return (await _categoryService.GetCategoryById(dto.Id), false); + return (await _categoryService.GetCategoryById(dto.Id), false, null); if (mode == ImportMode.Panel) - return (null, false); + return (null, false, null); if (!string.IsNullOrEmpty(dto.SeName)) - return (await FindCategoryBySlug(dto.SeName), false); + return (await ImportRows.FindBySlug(_slugService, dto.SeName, EntityTypes.Category, + _categoryService.GetCategoryById), false, null); if (string.IsNullOrEmpty(dto.Name)) - return (null, false); + return (null, false, null); - var found = await FindCategoryBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); - var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase) && - (string.IsNullOrEmpty(dto.ParentCategoryId) || dto.ParentCategoryId == found.ParentCategoryId); - return matches ? (found, true) : (null, false); + var parentId = string.IsNullOrEmpty(dto.ParentCategoryId) ? null : dto.ParentCategoryId; + var candidates = await _categoryService.GetAllCategories(parentId, dto.Name, "", showHidden: true); + var (found, error) = ImportRows.SingleByName(candidates, c => c.Name, dto.Name, "category"); + return (found, found != null, error); } - private async Task FindCategoryBySlug(string slug) - { - var url = await _slugService.GetBySlug(slug); - if (url is not { EntityName: EntityTypes.Category } || string.IsNullOrEmpty(url.EntityId)) - return null; - - return await _categoryService.GetCategoryById(url.EntityId); - } - - private async Task UpdateCategoryData(CategoryDto categoryDto, Category category) + private async Task UpdateCategoryData(CategoryDto categoryDto, Category category, bool parentMissing) { if (string.IsNullOrEmpty(category.CategoryLayoutId)) { @@ -193,12 +166,13 @@ private async Task UpdateCategoryData(CategoryDto categoryDto, Category category category.CategoryLayoutId = (await _categoryLayoutService.GetAllCategoryLayouts()).FirstOrDefault()?.Id; } - if (!string.IsNullOrEmpty(category.ParentCategoryId)) - { - var parentCategory = await _categoryService.GetCategoryById(category.ParentCategoryId); - if (parentCategory == null) - category.ParentCategoryId = string.Empty; - } + //the row's own parent was looked up already; only a stored one it did not replace is read here + if (parentMissing) + category.ParentCategoryId = string.Empty; + else if (!string.IsNullOrEmpty(category.ParentCategoryId) && + category.ParentCategoryId != categoryDto.ParentCategoryId && + await _categoryService.GetCategoryById(category.ParentCategoryId) == null) + category.ParentCategoryId = string.Empty; if (!string.IsNullOrEmpty(categoryDto.Picture)) { diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs index dea27b1403..dac6039853 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs @@ -1,4 +1,4 @@ -using Grand.Business.Core.Dto; +using Grand.Business.Core.Dto; using Grand.Business.Core.Interfaces.Catalog.Collections; using Grand.Business.Core.Interfaces.Common.Seo; using Grand.Business.Core.Interfaces.ExportImport; @@ -22,7 +22,6 @@ public class CollectionImportDataObject : IImportDataObject, IRow private readonly ISeNameService _seNameService; private readonly SecurityConfig _securityConfig; private readonly ImportHtmlGuard _htmlGuard; - private readonly SeoSettings _seoSettings; private readonly ILogger _logger; public CollectionImportDataObject( ICollectionService collectionService, @@ -32,7 +31,6 @@ public CollectionImportDataObject( ISeNameService seNameService, SecurityConfig securityConfig, ImportHtmlGuard htmlGuard, - SeoSettings seoSettings, ILogger logger) { _collectionService = collectionService; @@ -43,7 +41,6 @@ public CollectionImportDataObject( _seNameService = seNameService; _securityConfig = securityConfig; _htmlGuard = htmlGuard; - _seoSettings = seoSettings; _logger = logger; } @@ -60,32 +57,12 @@ public Task Import(IReadOnlyList rows, bool dr return Import(rows, dryRun, ImportMode.Row, cancellationToken); } - /// - /// Row: unknown Id rejected, SeName matching. Panel (XLSX): unknown Id creates the entity with that Id, no SeName matching. - /// - private enum ImportMode - { - Row, - Panel - } - - private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + /// Row: unknown Id rejected, SeName/Name matching. Panel (XLSX): unknown Id creates the entity with that Id, no matching. + private Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun, mode)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, (row, dto) => ImportRow(row, dto, dryRun, mode), + mode == ImportMode.Row, _logger, cancellationToken); } private async Task ImportRow(int row, CollectionDto dto, bool dryRun, ImportMode mode) @@ -94,7 +71,9 @@ private async Task ImportRow(int row, CollectionDto dto, bool d var warnings = new List(); var key = Key(dto); - var (existing, matchedByName) = await FindCollection(dto, mode); + var (existing, matchedByName, matchError) = await FindCollection(dto, mode); + if (matchError != null) + errors.Add(matchError); if (matchedByName) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) @@ -144,35 +123,28 @@ private static string Key(CollectionDto dto) } /// - /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by the slug - /// its Name would get - accepted only when the stored name is the same (any case) + /// Resolves the existing entity by Id; in Row mode, when no Id is given, by SeName, and with neither, by its stored + /// name (any case) - more than one collection of that name is an error /// - private async Task<(Collection collection, bool matchedByName)> FindCollection(CollectionDto dto, ImportMode mode) + private async Task<(Collection collection, bool matchedByName, string error)> FindCollection(CollectionDto dto, + ImportMode mode) { if (!string.IsNullOrEmpty(dto.Id)) - return (await _collectionService.GetCollectionById(dto.Id), false); + return (await _collectionService.GetCollectionById(dto.Id), false, null); if (mode == ImportMode.Panel) - return (null, false); + return (null, false, null); if (!string.IsNullOrEmpty(dto.SeName)) - return (await FindCollectionBySlug(dto.SeName), false); + return (await ImportRows.FindBySlug(_slugService, dto.SeName, EntityTypes.Collection, + _collectionService.GetCollectionById), false, null); if (string.IsNullOrEmpty(dto.Name)) - return (null, false); - - var found = await FindCollectionBySlug(ImportRows.NameSlug(dto.Name, _seoSettings)); - var matches = found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase); - return matches ? (found, true) : (null, false); - } - - private async Task FindCollectionBySlug(string slug) - { - var url = await _slugService.GetBySlug(slug); - if (url is not { EntityName: EntityTypes.Collection } || string.IsNullOrEmpty(url.EntityId)) - return null; + return (null, false, null); - return await _collectionService.GetCollectionById(url.EntityId); + var candidates = await _collectionService.GetAllCollections(dto.Name, "", showHidden: true); + var (found, error) = ImportRows.SingleByName(candidates, c => c.Name, dto.Name, "collection"); + return (found, found != null, error); } private async Task UpdateCollectionData(CollectionDto collectionDto, Collection collection) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs index be1ed56393..f82838b9b6 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ImportRows.cs @@ -1,20 +1,83 @@ -using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Common.Seo; using Grand.Business.Core.Interfaces.ExportImport; -using Grand.Domain.Seo; -using Grand.SharedKernel.Extensions; using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; +/// +/// Row: unknown Id rejected, matching by SeName/Sku/Name. Panel (XLSX): an unknown Id creates the entity with that Id. +/// +internal enum ImportMode +{ + Row, + Panel +} + /// Helpers the catalogue row imports share internal static class ImportRows { - /// The slug a new category, brand or collection gets from its name (as ISeNameService.ValidateSeName builds it) - public static string NameSlug(string name, SeoSettings seoSettings) + /// + /// Runs the rows one by one. A cancelled token stops the batch at a row boundary: what was saved is reported, the + /// rest is not touched. With a row that throws is logged and reported rejected + /// instead of losing the results of the rows saved before it; the panel's spreadsheet import leaves it off, so its + /// error still reaches the panel. + /// + public static async Task RunBatch(IReadOnlyList rows, bool dryRun, + Func id, Func key, Func> importRow, + bool isolateFailures, ILogger logger, CancellationToken cancellationToken) + { + var results = new List(rows.Count); + for (var i = 0; i < rows.Count; i++) + { + if (cancellationToken.IsCancellationRequested) + { + results.Add(ImportRowResult.NotProcessed(i + 1, id(rows[i]), key(rows[i]))); + continue; + } + + try + { + results.Add(await importRow(i + 1, rows[i])); + } + catch (Exception ex) when (isolateFailures) + { + logger.LogError(ex, "Import of row {Row} ({Key}) failed", i + 1, key(rows[i])); + results.Add(ImportRowResult.Failed(i + 1, id(rows[i]), key(rows[i]))); + } + } + + return new ImportBatchResult(dryRun, results); + } + + /// + /// The entity a slug points to, when it is of . A slug - active or a past one - is + /// reserved for one entity (ISeNameService.ValidateSeName), so it never resolves to two. + /// + public static async Task FindBySlug(ISlugService slugService, string slug, string entityName, + Func> getById) where T : class + { + var url = await slugService.GetBySlug(slug); + if (url == null || url.EntityName != entityName || string.IsNullOrEmpty(url.EntityId)) + return null; + + return await getById(url.EntityId); + } + + /// + /// The one candidate whose stored name is the row's name (any case). The candidates come from the service's + /// "name contains" filter, so a de-duplicated slug (apple-2) does not hide the entity. More than one is an error: + /// the row has to name the entity by Id or SeName. + /// + public static (T match, string error) SingleByName(IEnumerable candidates, Func name, + string wanted, string entity) where T : class { - var slug = SeoExtensions.GenerateSlug(name, seoSettings.ConvertNonWesternChars, - seoSettings.AllowUnicodeCharsInUrls, seoSettings.AllowSlashChar, seoSettings.SeoCharConversion); - return CommonHelper.EnsureMaximumLength(slug, 200); + var matches = candidates.Where(c => string.Equals(name(c), wanted, StringComparison.OrdinalIgnoreCase)) + .Take(2).ToList(); + return matches.Count switch { + 0 => (null, null), + 1 => (matches[0], null), + _ => (null, $"Name '{wanted}' matches more than one {entity}; send Id or SeName.") + }; } public static string MatchedByNameWarning(string name) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs index 3bdf232ffc..7386129906 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs @@ -4,6 +4,7 @@ using Grand.Business.Core.Interfaces.ExportImport; using Grand.Domain.Catalog; using Grand.Domain.Seo; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; @@ -11,12 +12,14 @@ namespace Grand.Business.Catalog.Services.ExportImport; public class ProductAttributeImportDataObject : IRowImport { private readonly ImportHtmlGuard _htmlGuard; + private readonly ILogger _logger; private readonly IProductAttributeService _productAttributeService; private readonly SeoSettings _seoSettings; public ProductAttributeImportDataObject(IProductAttributeService productAttributeService, - SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + SeoSettings seoSettings, ImportHtmlGuard htmlGuard, ILogger logger) { + _logger = logger; _productAttributeService = productAttributeService; _seoSettings = seoSettings; _htmlGuard = htmlGuard; @@ -25,29 +28,20 @@ public ProductAttributeImportDataObject(IProductAttributeService productAttribut public async Task Import(IReadOnlyList rows, bool dryRun, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun)); - } - - return new ImportBatchResult(dryRun, results); + //there is no lookup by SeName: the attributes are loaded once per batch, and the ones it creates are added + var all = (await _productAttributeService.GetAllProductAttributes()).ToList(); + return await ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, (row, dto) => ImportRow(row, dto, dryRun, all), + true, _logger, cancellationToken); } - private async Task ImportRow(int row, ProductAttributeDto dto, bool dryRun) + private async Task ImportRow(int row, ProductAttributeDto dto, bool dryRun, + List all) { var errors = new List(); var warnings = new List(); var key = Key(dto); - var (existing, matchedByName) = await Find(dto); + var (existing, matchedByName) = await Find(dto, all); if (matchedByName) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null) @@ -70,6 +64,12 @@ private async Task ImportRow(int row, ProductAttributeDto dto, errors.AddRange(_htmlGuard.PlainTextErrors(("Value Name", value.Name))); } + var seName = existing == null || !string.IsNullOrEmpty(dto.SeName) + ? GetSeName(string.IsNullOrEmpty(dto.SeName) ? dto.Name : dto.SeName) + : null; + if (!string.IsNullOrEmpty(seName) && FindBySeName(all, seName) is { } owner && owner.Id != existing?.Id) + errors.Add($"SeName '{seName}' is already used by '{owner.Name}'."); + if (errors.Count > 0) return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); @@ -81,8 +81,7 @@ private async Task ImportRow(int row, ProductAttributeDto dto, var attribute = existing ?? new ProductAttribute { Name = dto.Name }; if (!string.IsNullOrEmpty(dto.Name)) attribute.Name = dto.Name; if (dto.Description != null) attribute.Description = dto.Description; - if (isNew || !string.IsNullOrEmpty(dto.SeName)) - attribute.SeName = GetSeName(string.IsNullOrEmpty(dto.SeName) ? attribute.Name : dto.SeName); + if (seName != null) attribute.SeName = seName; //a repeated name in the same row matches the value just added, so it collapses into it foreach (var valueDto in dto.Values ?? []) @@ -102,8 +101,15 @@ private async Task ImportRow(int row, ProductAttributeDto dto, if (valueDto.DisplayOrder.HasValue) value.DisplayOrder = valueDto.DisplayOrder.Value; } - if (isNew) await _productAttributeService.InsertProductAttribute(attribute); - else await _productAttributeService.UpdateProductAttribute(attribute); + if (isNew) + { + await _productAttributeService.InsertProductAttribute(attribute); + all.Add(attribute); + } + else + { + await _productAttributeService.UpdateProductAttribute(attribute); + } return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, attribute.Id, key, errors, warnings); @@ -118,31 +124,29 @@ private static string Key(ProductAttributeDto dto) /// By Id, else by SeName normalized as a new attribute's is, else (best effort) by the SeName its Name would get, /// accepted only when the stored name is the same (any case) /// - private async Task<(ProductAttribute attribute, bool matchedByName)> Find(ProductAttributeDto dto) + private async Task<(ProductAttribute attribute, bool matchedByName)> Find(ProductAttributeDto dto, + List all) { if (!string.IsNullOrEmpty(dto.Id)) return (await _productAttributeService.GetProductAttributeById(dto.Id), false); if (!string.IsNullOrEmpty(dto.SeName)) - return (await FindBySeName(GetSeName(dto.SeName)), false); + return (FindBySeName(all, GetSeName(dto.SeName)), false); if (string.IsNullOrEmpty(dto.Name)) return (null, false); - var found = await FindBySeName(GetSeName(dto.Name)); + var found = FindBySeName(all, GetSeName(dto.Name)); return found != null && string.Equals(found.Name, dto.Name, StringComparison.OrdinalIgnoreCase) ? (found, true) : (null, false); } - //no lookup by SeName exists: load all and match - private async Task FindBySeName(string seName) + private static ProductAttribute FindBySeName(List all, string seName) { - if (string.IsNullOrEmpty(seName)) - return null; - - var all = await _productAttributeService.GetAllProductAttributes(); - return all.FirstOrDefault(a => string.Equals(a.SeName, seName, StringComparison.OrdinalIgnoreCase)); + return string.IsNullOrEmpty(seName) + ? null + : all.FirstOrDefault(a => string.Equals(a.SeName, seName, StringComparison.OrdinalIgnoreCase)); } //same normalization as the panel's product attribute controller diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs index 72541ee838..4a51a7956a 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductImportDataObject.cs @@ -103,40 +103,26 @@ public Task Import(IReadOnlyList rows, bool dryRu return Import(rows, dryRun, ImportMode.Row, cancellationToken); } - /// - /// Row: unknown Id rejected, Sku matching only when no Id is given. Panel (XLSX): by Id, then by Sku; an unknown Id - /// creates the product with that Id. - /// - private enum ImportMode - { - Row, - Panel - } - private sealed class PictureBudget { public int Downloads; } - private async Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, + /// The brand, categories and collections of the row that exist, read once per row + private sealed record References(bool BrandMissing, List CategoryIds, List CollectionIds); + + /// + /// Row: unknown Id rejected, Sku matching only when no Id is given. Panel (XLSX): by Id, then by Sku; an unknown Id + /// creates the product with that Id. + /// + private Task Import(IReadOnlyList rows, bool dryRun, ImportMode mode, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); //the cap protects the row API; the panel's spreadsheet import downloads every picture var budget = mode == ImportMode.Row ? new PictureBudget() : null; - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun, mode, budget)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, + (row, dto) => ImportRow(row, dto, dryRun, mode, budget), mode == ImportMode.Row, _logger, + cancellationToken); } private async Task ImportRow(int row, ProductDto dto, bool dryRun, ImportMode mode, @@ -165,7 +151,12 @@ private async Task ImportRow(int row, ProductDto dto, bool dryR (nameof(dto.MetaKeywords), dto.MetaKeywords), (nameof(dto.MetaDescription), dto.MetaDescription), (nameof(dto.MetaTitle), dto.MetaTitle), (nameof(dto.Sku), dto.Sku))); - await AddReferenceWarnings(dto, warnings); + //Sku is what a row without Id is matched on, so the row API keeps it unique + if (mode == ImportMode.Row && existing != null && !string.IsNullOrEmpty(dto.Sku) && + await _productService.GetProductBySku(dto.Sku) is { } skuOwner && skuOwner.Id != existing.Id) + errors.Add($"Sku '{dto.Sku}' is already used by product '{skuOwner.Name}'."); + + var references = await ResolveReferences(dto, warnings); //the Sku is what a later call matches on; without one the row cannot be found again if (mode == ImportMode.Row && existing == null && string.IsNullOrEmpty(dto.Sku)) warnings.Add("Row has no Sku; sending it again creates a duplicate."); @@ -200,7 +191,7 @@ private async Task ImportRow(int row, ProductDto dto, bool dryR if (isNew) await _productService.InsertProduct(product); else await _productService.UpdateProduct(product); - await UpdateProductData(dto, product, isNew, budget, warnings); + await UpdateProductData(dto, product, isNew, references, budget, warnings); return new ImportRowResult(row, isNew ? ImportRowStatus.Created : ImportRowStatus.Updated, product.Id, key, errors, warnings); } @@ -218,6 +209,11 @@ private void ApplyCreateDefaults(ProductDto dto, Product product) if (!dto.NotifyAdminForQuantityBelow.HasValue) product.NotifyAdminForQuantityBelow = 1; if (!dto.IsShipEnabled.HasValue) product.IsShipEnabled = true; if (!dto.AllowCustomerReviews.HasValue) product.AllowCustomerReviews = true; + if (!dto.MaxEnteredPrice.HasValue) product.MaxEnteredPrice = 1000; + if (!dto.UnlimitedDownloads.HasValue) product.UnlimitedDownloads = true; + if (!dto.MaxNumberOfDownloads.HasValue) product.MaxNumberOfDownloads = 10; + if (!dto.RecurringCycleLength.HasValue) product.RecurringCycleLength = 100; + if (!dto.RecurringTotalCycles.HasValue) product.RecurringTotalCycles = 10; if (dto.TaxCategoryId == null) product.TaxCategoryId = _taxSettings.DefaultTaxCategoryId; } @@ -227,18 +223,27 @@ private static string Key(ProductDto dto) } /// Ids that do not exist are skipped (categories, collections) or cleared (brand); say so - private async Task AddReferenceWarnings(ProductDto dto, List warnings) + private async Task ResolveReferences(ProductDto dto, List warnings) { - if (!string.IsNullOrEmpty(dto.BrandId) && await _brandService.GetBrandById(dto.BrandId) == null) + var brandMissing = !string.IsNullOrEmpty(dto.BrandId) && await _brandService.GetBrandById(dto.BrandId) == null; + if (brandMissing) warnings.Add($"BrandId '{dto.BrandId}' was not found and was cleared."); + var categoryIds = new List(); foreach (var id in SplitIds(dto.CategoryIds)) if (await _categoryService.GetCategoryById(id) == null) warnings.Add($"Category id '{id}' was not found and was skipped."); + else + categoryIds.Add(id); + var collectionIds = new List(); foreach (var id in SplitIds(dto.CollectionIds)) if (await _collectionService.GetCollectionById(id) == null) warnings.Add($"Collection id '{id}' was not found and was skipped."); + else + collectionIds.Add(id); + + return new References(brandMissing, categoryIds, collectionIds); } private static IEnumerable SplitIds(string ids) @@ -249,29 +254,25 @@ private static IEnumerable SplitIds(string ids) } private async Task UpdateProductData(ProductDto productDto, Product product, bool isNew, - PictureBudget budget, List warnings) + References references, PictureBudget budget, List warnings) { await UpdateProductDataLayout(product); await UpdateProductDataDeliveryDate(product); await UpdateProductDataTaxCategory(product); await UpdateProductDataWarehouse(product); await UpdateProductDataUnit(product); - await UpdateProductDataBrand(product); + await UpdateProductDataBrand(productDto, product, references); //search engine name var seName = product.SeName ?? product.Name; seName = await _seNameService.ValidateSeName(product, seName, product.Name, true); await _slugService.SaveSlug(product, seName, ""); product.SeName = seName; + //UpdateProduct also works out LowStock await _productService.UpdateProduct(product); - product.LowStock = product.MinStockQuantity > 0 && product.MinStockQuantity >= product.StockQuantity; - - if (!string.IsNullOrEmpty(productDto.CategoryIds)) - await PrepareProductCategories(product, productDto.CategoryIds); - - if (!string.IsNullOrEmpty(productDto.CollectionIds)) - await PrepareProductCollections(product, productDto.CollectionIds); + await PrepareProductCategories(product, references.CategoryIds); + await PrepareProductCollections(product, references.CollectionIds); //pictures await PrepareProductPictures(productDto, product, isNew, budget, warnings); @@ -323,25 +324,25 @@ private async Task UpdateProductDataUnit(Product product) product.UnitId = ""; } - private async Task UpdateProductDataBrand(Product product) + private async Task UpdateProductDataBrand(ProductDto productDto, Product product, References references) { if (string.IsNullOrEmpty(product.BrandId)) return; - var brand = await _brandService.GetBrandById(product.BrandId); - if (brand == null) + //the row's own brand was looked up already; only a stored one it did not replace is read here + var missing = product.BrandId == productDto.BrandId + ? references.BrandMissing + : await _brandService.GetBrandById(product.BrandId) == null; + if (missing) product.BrandId = ""; } - private async Task PrepareProductCategories(Product product, string categoryIds) + /// Ids already known to exist + private async Task PrepareProductCategories(Product product, List categoryIds) { - foreach (var id in categoryIds.Split([';'], StringSplitOptions.RemoveEmptyEntries) - .Select(x => x.Trim())) + foreach (var id in categoryIds) { if (product.ProductCategories.FirstOrDefault(x => x.CategoryId == id) != null) continue; - //ensure that category exists - var category = await _categoryService.GetCategoryById(id); - if (category == null) continue; var productCategory = new ProductCategory { - CategoryId = category.Id, + CategoryId = id, IsFeaturedProduct = false, DisplayOrder = 1 }; @@ -349,17 +350,14 @@ private async Task PrepareProductCategories(Product product, string categoryIds) } } - private async Task PrepareProductCollections(Product product, string collectionIds) + /// Ids already known to exist + private async Task PrepareProductCollections(Product product, List collectionIds) { - foreach (var id in collectionIds.Split([';'], StringSplitOptions.RemoveEmptyEntries) - .Select(x => x.Trim())) + foreach (var id in collectionIds) { if (product.ProductCollections.FirstOrDefault(x => x.CollectionId == id) != null) continue; - //ensure that collection exists - var collection = await _collectionService.GetCollectionById(id); - if (collection == null) continue; var productCollection = new ProductCollection { - CollectionId = collection.Id, + CollectionId = id, IsFeaturedProduct = false, DisplayOrder = 1 }; diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs index 69ab447efe..5ce19df6a9 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs @@ -5,6 +5,7 @@ using Grand.Domain.Catalog; using Grand.Domain.Common; using Grand.Domain.Seo; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; @@ -19,36 +20,32 @@ public class ProductVariantImportDataObject : IRowImport public const int MaxCombinationsPerProduct = 50; private readonly ImportHtmlGuard _htmlGuard; + private readonly ILogger _logger; private readonly IProductAttributeService _productAttributeService; private readonly IProductService _productService; private readonly SeoSettings _seoSettings; public ProductVariantImportDataObject(IProductService productService, - IProductAttributeService productAttributeService, SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + IProductAttributeService productAttributeService, SeoSettings seoSettings, ImportHtmlGuard htmlGuard, + ILogger logger) { + _logger = logger; _productService = productService; _productAttributeService = productAttributeService; _seoSettings = seoSettings; _htmlGuard = htmlGuard; } - public async Task Import(IReadOnlyList rows, bool dryRun, + /// + /// The writes of one row are not atomic: a row that fails part way comes back with + /// . Sending it again is safe - mappings, values and combinations are + /// matched, not added twice - and completes it + /// + public Task Import(IReadOnlyList rows, bool dryRun, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].ProductId, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.ProductId, Key, (row, dto) => ImportRow(row, dto, dryRun), + true, _logger, cancellationToken); } private async Task ImportRow(int row, ProductVariantsDto dto, bool dryRun) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs index f90a529506..91971474a7 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs @@ -4,6 +4,7 @@ using Grand.Business.Core.Interfaces.ExportImport; using Grand.Domain.Catalog; using Grand.Domain.Seo; +using Microsoft.Extensions.Logging; namespace Grand.Business.Catalog.Services.ExportImport; @@ -11,34 +12,24 @@ namespace Grand.Business.Catalog.Services.ExportImport; public class SpecificationAttributeImportDataObject : IRowImport { private readonly ImportHtmlGuard _htmlGuard; + private readonly ILogger _logger; private readonly SeoSettings _seoSettings; private readonly ISpecificationAttributeService _specificationAttributeService; public SpecificationAttributeImportDataObject(ISpecificationAttributeService specificationAttributeService, - SeoSettings seoSettings, ImportHtmlGuard htmlGuard) + SeoSettings seoSettings, ImportHtmlGuard htmlGuard, ILogger logger) { + _logger = logger; _specificationAttributeService = specificationAttributeService; _seoSettings = seoSettings; _htmlGuard = htmlGuard; } - public async Task Import(IReadOnlyList rows, bool dryRun, + public Task Import(IReadOnlyList rows, bool dryRun, CancellationToken cancellationToken = default) { - var results = new List(rows.Count); - for (var i = 0; i < rows.Count; i++) - { - //a batch that runs out of time stops at a row boundary: what was saved is reported, the rest is not touched - if (cancellationToken.IsCancellationRequested) - { - results.Add(ImportRowResult.NotProcessed(i + 1, rows[i].Id, Key(rows[i]))); - continue; - } - - results.Add(await ImportRow(i + 1, rows[i], dryRun)); - } - - return new ImportBatchResult(dryRun, results); + return ImportRows.RunBatch(rows, dryRun, r => r.Id, Key, (row, dto) => ImportRow(row, dto, dryRun), true, + _logger, cancellationToken); } private async Task ImportRow(int row, SpecificationAttributeDto dto, bool dryRun) @@ -70,6 +61,13 @@ private async Task ImportRow(int row, SpecificationAttributeDto ("Option ColorSquaresRgb", option.ColorSquaresRgb))); } + //the storefront's specification filters find an attribute by SeName, so it has to stay unique + var seName = existing == null || !string.IsNullOrEmpty(dto.SeName) + ? GetSeName(string.IsNullOrEmpty(dto.SeName) ? dto.Name : dto.SeName) + : null; + if (!string.IsNullOrEmpty(seName) && await FindBySeName(seName) is { } owner && owner.Id != existing?.Id) + errors.Add($"SeName '{seName}' is already used by '{owner.Name}'."); + if (errors.Count > 0) return new ImportRowResult(row, ImportRowStatus.Rejected, existing?.Id ?? "", key, errors, warnings); @@ -81,8 +79,7 @@ private async Task ImportRow(int row, SpecificationAttributeDto var attribute = existing ?? new SpecificationAttribute { Name = dto.Name }; if (!string.IsNullOrEmpty(dto.Name)) attribute.Name = dto.Name; if (dto.DisplayOrder.HasValue) attribute.DisplayOrder = dto.DisplayOrder.Value; - if (isNew || !string.IsNullOrEmpty(dto.SeName)) - attribute.SeName = GetSeName(string.IsNullOrEmpty(dto.SeName) ? attribute.Name : dto.SeName); + if (seName != null) attribute.SeName = seName; //a repeated name in the same row matches the option just added, so it collapses into it foreach (var optionDto in dto.Options ?? []) diff --git a/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs b/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs index 8b48e26bad..093b36bade 100644 --- a/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs +++ b/src/Business/Grand.Business.Core/Interfaces/ExportImport/IRowImport.cs @@ -20,6 +20,15 @@ public static ImportRowResult NotProcessed(int row, string id, string key) { return new ImportRowResult(row, ImportRowStatus.Rejected, id ?? "", key, [NotProcessedError], []); } + + public const string FailedError = + "The row failed while it was being saved and may be partly written; the error is in the log. Send the row again."; + + /// A row that threw; the batch went on with the next row + public static ImportRowResult Failed(int row, string id, string key) + { + return new ImportRowResult(row, ImportRowStatus.Rejected, id ?? "", key, [FailedError], []); + } } public sealed record ImportBatchResult(bool DryRun, IReadOnlyList Rows) @@ -31,7 +40,9 @@ public sealed record ImportBatchResult(bool DryRun, IReadOnlyList /// Row-level import with validation and dry run; the panel's Execute runs it with dryRun = false. A cancelled -/// token does not throw: the rows not reached yet come back rejected with +/// token does not throw: the rows not reached yet come back rejected with . +/// Neither does a row that fails while saving: it comes back rejected with +/// and the batch goes on /// public interface IRowImport { diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs index 6a3e0c5550..7f6f16ab42 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/AttributeImportDataObjectTests.cs @@ -6,6 +6,7 @@ using Grand.Domain.Seo; using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Security; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -42,7 +43,8 @@ public void Init() _specificationAttributeService.Setup(s => s.UpdateSpecificationAttribute(It.IsAny())) .Callback(() => _writes++).Returns(Task.CompletedTask); _specificationImport = - new SpecificationAttributeImportDataObject(_specificationAttributeService.Object, new SeoSettings(), guard); + new SpecificationAttributeImportDataObject(_specificationAttributeService.Object, new SeoSettings(), guard, + new Mock>().Object); _productAttributeService = new Mock(); _productAttributeService.Setup(s => s.GetProductAttributeById(It.IsAny())) @@ -53,7 +55,8 @@ public void Init() .Callback((ProductAttribute a) => { _writes++; _productAttributes.Add(a); }).Returns(Task.CompletedTask); _productAttributeService.Setup(s => s.UpdateProductAttribute(It.IsAny())) .Callback(() => _writes++).Returns(Task.CompletedTask); - _productImport = new ProductAttributeImportDataObject(_productAttributeService.Object, new SeoSettings(), guard); + _productImport = new ProductAttributeImportDataObject(_productAttributeService.Object, new SeoSettings(), guard, + new Mock>().Object); } [TestMethod] @@ -335,12 +338,72 @@ public async Task Product_NameOnlyResent_MatchesExistingByName(bool dryRun) } [TestMethod] - public async Task Specification_SlugOfDifferentlyNamedAttribute_IsNotMatchedByName() + [DataRow(true)] + [DataRow(false)] + public async Task Specification_SlugOfDifferentlyNamedAttribute_IsRejectedAsSeNameTaken(bool dryRun) { _specificationAttributes.Add(new SpecificationAttribute { Id = "1", Name = "Shirt-Size", SeName = "shirt-size" }); - var result = await _specificationImport.Import([new SpecificationAttributeDto { Name = "Shirt Size" }], true); + var result = await _specificationImport.Import([new SpecificationAttributeDto { Name = "Shirt Size" }], dryRun); - Assert.AreEqual(ImportRowStatus.Created, result.Rows[0].Status); + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("SeName 'shirt-size' is already used by 'Shirt-Size'.", result.Rows[0].Errors); + Assert.AreEqual(1, _specificationAttributes.Count); + } + + [TestMethod] + public async Task Specification_UpdateByIdToAnotherAttributesSeName_IsRejected() + { + _specificationAttributes.Add(new SpecificationAttribute { Id = "1", Name = "Color", SeName = "color" }); + _specificationAttributes.Add(new SpecificationAttribute { Id = "2", Name = "Size", SeName = "size" }); + + var result = await _specificationImport.Import([new SpecificationAttributeDto { Id = "2", SeName = "Color" }], + false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.AreEqual("size", _specificationAttributes[1].SeName); + Assert.AreEqual(0, _writes); + } + + [TestMethod] + public async Task Product_SlugOfDifferentlyNamedAttribute_IsRejectedAsSeNameTaken() + { + _productAttributes.Add(new ProductAttribute { Id = "7", Name = "Gift-Wrap", SeName = "gift-wrap" }); + + var result = await _productImport.Import([new ProductAttributeDto { Name = "Gift Wrap" }], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("SeName 'gift-wrap' is already used by 'Gift-Wrap'.", result.Rows[0].Errors); + Assert.AreEqual(1, _productAttributes.Count); + } + + [TestMethod] + public async Task Product_AttributesAreLoadedOncePerBatch() + { + var result = await _productImport.Import([ + new ProductAttributeDto { Name = "Gift Wrap" }, new ProductAttributeDto { Name = "gift wrap" }, + new ProductAttributeDto { SeName = "gift-wrap" } + ], false); + + Assert.AreEqual(1, result.Created); + Assert.AreEqual(2, result.Updated); + _productAttributeService.Verify( + s => s.GetAllProductAttributes(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Specification_RowThatThrows_IsRejectedAndBatchContinues() + { + _specificationAttributeService.Setup(s => s.InsertSpecificationAttribute( + It.Is(a => a.Name == "Broken"))) + .ThrowsAsync(new InvalidOperationException("boom")); + + var result = await _specificationImport.Import([ + new SpecificationAttributeDto { Name = "Broken" }, new SpecificationAttributeDto { Name = "Material" } + ], false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains(ImportRowResult.FailedError, result.Rows[0].Errors); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); } } diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs index cab6445815..dc3c93e257 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/BrandImportDataObjectTests.cs @@ -72,7 +72,7 @@ public void Init() _brandImportDataObject = new BrandImportDataObject(_brandService, _pictureServiceMock.Object, _brandLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), - new SeoSettings(), (_loggerMock = new Mock>()).Object); + (_loggerMock = new Mock>()).Object); } [TestMethod] @@ -444,6 +444,37 @@ public async Task Import_NameOnlyResent_MatchesExistingByName(bool dryRun) Assert.AreEqual(dryRun ? 0 : 7, _repository.Table.Single().DisplayOrder); } + [TestMethod] + public async Task Import_NameOnly_MatchesBrandWhoseSlugWasRenumbered() + { + SetupLayouts(); + var existing = new Brand { Name = "Apple", SeName = "apple-2" }; + await _brandService.InsertBrand(existing); + + var first = await _brandImportDataObject.Import(new List { new() { Name = "Apple" } }, false); + var second = await _brandImportDataObject.Import(new List { new() { Name = "Apple" } }, false); + + Assert.AreEqual(existing.Id, first.Rows[0].Id); + Assert.AreEqual(existing.Id, second.Rows[0].Id); + Assert.AreEqual(1, _repository.Table.Count()); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_NameOnlyMatchingTwoBrands_IsRejected(bool dryRun) + { + SetupLayouts(); + await _brandService.InsertBrand(new Brand { Name = "Apple", SeName = "apple" }); + await _brandService.InsertBrand(new Brand { Name = "APPLE", SeName = "apple-1" }); + + var result = await _brandImportDataObject.Import(new List { new() { Name = "apple" } }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Name 'apple' matches more than one brand; send Id or SeName.", result.Rows[0].Errors); + Assert.AreEqual(2, _repository.Table.Count()); + } + private void InitAutoMapper() { var typeSearcher = new TypeSearcher(); diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs index d95be9e0fc..43fb3a9929 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CategoryImportDataObjectTests.cs @@ -73,7 +73,7 @@ public void Init() _categoryImportDataObject = new CategoryImportDataObject(_categoryService, _pictureServiceMock.Object, _categoryLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), - new SeoSettings(), (_loggerMock = new Mock>()).Object); + (_loggerMock = new Mock>()).Object); } [TestMethod] @@ -470,6 +470,26 @@ public async Task Import_NameOnlyResent_MatchesExistingByName(bool dryRun) Assert.AreEqual(dryRun ? 0 : 7, _repository.Table.Single().DisplayOrder); } + [TestMethod] + public async Task Import_NameOnlyMatchingCategoriesUnderTwoParents_IsRejected() + { + SetupLayouts(); + var men = new Category { Name = "Men" }; + await _categoryService.InsertCategory(men); + var women = new Category { Name = "Women" }; + await _categoryService.InsertCategory(women); + await _categoryService.InsertCategory(new Category { Name = "Shoes", ParentCategoryId = men.Id }); + await _categoryService.InsertCategory(new Category { Name = "Shoes", ParentCategoryId = women.Id }); + + var ambiguous = await _categoryImportDataObject.Import(new List { new() { Name = "Shoes" } }, true); + var underWomen = await _categoryImportDataObject.Import( + new List { new() { Name = "Shoes", ParentCategoryId = women.Id } }, true); + + Assert.AreEqual(ImportRowStatus.Rejected, ambiguous.Rows[0].Status); + Assert.Contains("Name 'Shoes' matches more than one category; send Id or SeName.", ambiguous.Rows[0].Errors); + Assert.AreEqual(ImportRowStatus.Updated, underWomen.Rows[0].Status); + } + [TestMethod] public async Task Import_SameNameUnderAnotherParent_IsCreated() { diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs index ceb0f8bf76..4fb9eef77b 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/CollectionImportDataObjectTests.cs @@ -73,7 +73,7 @@ public void Init() _collectionImportDataObject = new CollectionImportDataObject(_collectionService, _pictureServiceMock.Object, _collectionLayoutServiceMock.Object, _slugServiceMock.Object, _seNameService, _securityConfig, new ImportHtmlGuard(new HtmlSanitizationService(_securityConfig), _securityConfig), - new SeoSettings(), (_loggerMock = new Mock>()).Object); + (_loggerMock = new Mock>()).Object); } [TestMethod] diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs index b70354c24b..08740cab39 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductImportDataObjectTests.cs @@ -697,9 +697,99 @@ public async Task Import_NewRow_GetsThePanelCreateDefaults() Assert.IsTrue(product.IsShipEnabled); Assert.IsTrue(product.AllowCustomerReviews); Assert.AreEqual("tax-default", product.TaxCategoryId); + Assert.AreEqual(1000, product.MaxEnteredPrice); + Assert.IsTrue(product.UnlimitedDownloads); + Assert.AreEqual(10, product.MaxNumberOfDownloads); + Assert.AreEqual(100, product.RecurringCycleLength); + Assert.AreEqual(10, product.RecurringTotalCycles); Assert.IsFalse(product.Published); } + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task Import_UpdateByIdToAnotherProductsSku_IsRejected(bool dryRun) + { + SetupLookups(); + var first = new Product { Name = "first", Sku = "A1" }; + var second = new Product { Name = "second", Sku = "B1" }; + await _productService.InsertProduct(first); + await _productService.InsertProduct(second); + + var result = await _productImportDataObject.Import(new List { + new() { Id = second.Id, Sku = "A1" } + }, dryRun); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains("Sku 'A1' is already used by product 'first'.", result.Rows[0].Errors); + Assert.AreEqual("B1", (await _repository.GetByIdAsync(second.Id)).Sku); + } + + [TestMethod] + public async Task Import_UpdateByIdKeepingOwnSku_IsUpdated() + { + SetupLookups(); + var product = new Product { Name = "first", Sku = "A1" }; + await _productService.InsertProduct(product); + + var result = await _productImportDataObject.Import(new List { + new() { Id = product.Id, Sku = "A1", Name = "renamed" } + }, false); + + Assert.AreEqual(ImportRowStatus.Updated, result.Rows[0].Status); + } + + [TestMethod] + public async Task Import_RowThatThrows_IsRejectedAndLaterRowsAreSaved() + { + SetupLookups(); + _categoryServiceMock.Setup(c => c.GetCategoryById("cat")).ReturnsAsync(new Category { Id = "cat" }); + _productCategoryServiceMock + .Setup(c => c.InsertProductCategory(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("boom")); + + var result = await _productImportDataObject.Import(new List { + new() { Name = "broken", Sku = "X1", CategoryIds = "cat" }, + new() { Name = "fine", Sku = "X2" } + }, false); + + Assert.AreEqual(ImportRowStatus.Rejected, result.Rows[0].Status); + Assert.Contains(ImportRowResult.FailedError, result.Rows[0].Errors); + Assert.AreEqual(ImportRowStatus.Created, result.Rows[1].Status); + Assert.AreEqual(2, _repository.Table.Count()); + } + + [TestMethod] + public async Task Execute_RowThatThrows_StillThrows() + { + SetupLookups(); + _categoryServiceMock.Setup(c => c.GetCategoryById("cat")).ReturnsAsync(new Category { Id = "cat" }); + _productCategoryServiceMock + .Setup(c => c.InsertProductCategory(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("boom")); + + await Assert.ThrowsExactlyAsync(() => + _productImportDataObject.Execute(new List { new() { Name = "broken", CategoryIds = "cat" } })); + } + + [TestMethod] + public async Task Import_ReferencesAreReadOncePerRow() + { + SetupLookups(); + _categoryServiceMock.Setup(c => c.GetCategoryById("cat")).ReturnsAsync(new Category { Id = "cat" }); + _brandServiceMock.Setup(c => c.GetBrandById("brand")).ReturnsAsync(new Brand { Id = "brand" }); + + await _productImportDataObject.Import(new List { + new() { Name = "p", CategoryIds = "cat", BrandId = "brand" } + }, false); + + _categoryServiceMock.Verify(c => c.GetCategoryById("cat"), Times.Once); + _brandServiceMock.Verify(c => c.GetBrandById("brand"), Times.Once); + _productCategoryServiceMock.Verify( + c => c.InsertProductCategory(It.Is(pc => pc.CategoryId == "cat"), It.IsAny()), + Times.Once); + } + [TestMethod] public async Task Import_NewRow_ExplicitValuesWinOverCreateDefaults() { diff --git a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs index ea0d04ba7b..34dd6cd882 100644 --- a/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs +++ b/src/Tests/Grand.Business.Catalog.Tests/Services/ExportImport/ProductVariantImportDataObjectTests.cs @@ -12,6 +12,7 @@ using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Security; using Grand.Infrastructure.Tests.Caching; +using Microsoft.Extensions.Logging; using Grand.Mediator; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -60,7 +61,7 @@ public async Task Init() var securityConfig = new SecurityConfig(); var guard = new ImportHtmlGuard(new HtmlSanitizationService(securityConfig), securityConfig); _import = new ProductVariantImportDataObject(_productService.Object, _attributeService.Object, - new SeoSettings(), guard); + new SeoSettings(), guard, new Mock>().Object); } private int Writes => _attributeService.Invocations.Count(i => From 7866d01c4a3c80ce0ef60202c6741b20c7e2106e Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sat, 10 Oct 2026 22:03:37 +0200 Subject: [PATCH 6/6] Catalog import: make the null checks the code-quality analyzer asked for explicit --- .../Services/ExportImport/BrandImportDataObject.cs | 2 +- .../Services/ExportImport/CategoryImportDataObject.cs | 2 +- .../Services/ExportImport/CollectionImportDataObject.cs | 2 +- .../Services/ExportImport/ProductAttributeImportDataObject.cs | 2 +- .../Services/ExportImport/ProductVariantImportDataObject.cs | 2 +- .../ExportImport/SpecificationAttributeImportDataObject.cs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs index 934ad5569c..f08ad1f4bd 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/BrandImportDataObject.cs @@ -74,7 +74,7 @@ private async Task ImportRow(int row, BrandDto dto, bool dryRun var (existing, matchedByName, matchError) = await FindBrand(dto, mode); if (matchError != null) errors.Add(matchError); - if (matchedByName) + if (matchedByName && existing != null) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) errors.Add($"Id '{dto.Id}' was not found."); diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs index 21a7971242..913135768d 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CategoryImportDataObject.cs @@ -74,7 +74,7 @@ private async Task ImportRow(int row, CategoryDto dto, bool dry var (existing, matchedByName, matchError) = await FindCategory(dto, mode); if (matchError != null) errors.Add(matchError); - if (matchedByName) + if (matchedByName && existing != null) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) errors.Add($"Id '{dto.Id}' was not found."); diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs index dac6039853..b124d0cc98 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/CollectionImportDataObject.cs @@ -74,7 +74,7 @@ private async Task ImportRow(int row, CollectionDto dto, bool d var (existing, matchedByName, matchError) = await FindCollection(dto, mode); if (matchError != null) errors.Add(matchError); - if (matchedByName) + if (matchedByName && existing != null) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null && mode == ImportMode.Row) errors.Add($"Id '{dto.Id}' was not found."); diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs index 7386129906..5ba0dbb36f 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductAttributeImportDataObject.cs @@ -42,7 +42,7 @@ private async Task ImportRow(int row, ProductAttributeDto dto, var key = Key(dto); var (existing, matchedByName) = await Find(dto, all); - if (matchedByName) + if (matchedByName && existing != null) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null) errors.Add($"Id '{dto.Id}' was not found."); diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs index 5ce19df6a9..91d3311841 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/ProductVariantImportDataObject.cs @@ -92,7 +92,7 @@ private async Task ImportRow(int row, ProductVariantsDto dto, b var attributes = await ResolveAttributes(attributeDtos, combinationDtos, errors); - if (errors.Count > 0) + if (errors.Count > 0 || product == null) return new ImportRowResult(row, ImportRowStatus.Rejected, product?.Id ?? "", key, errors, warnings); //the product was read from the database, not the cache, so a dry run can change it in memory freely diff --git a/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs index 91971474a7..62965fe874 100644 --- a/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs +++ b/src/Business/Grand.Business.Catalog/Services/ExportImport/SpecificationAttributeImportDataObject.cs @@ -39,7 +39,7 @@ private async Task ImportRow(int row, SpecificationAttributeDto var key = Key(dto); var (existing, matchedByName) = await Find(dto); - if (matchedByName) + if (matchedByName && existing != null) warnings.Add(ImportRows.MatchedByNameWarning(existing.Name)); if (!string.IsNullOrEmpty(dto.Id) && existing == null) errors.Add($"Id '{dto.Id}' was not found.");