From 3ddc487b62f4e45ef0222ff7b3cfe55039cf6ef2 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 18:34:06 +0100 Subject: [PATCH 1/6] Keep e-mailed invoices in Downloads and delete them with the e-mail An invoice holds customer data, so it belongs in the application's file storage, with a lifetime tied to the e-mail it was generated for. Until now it depended on a setting, and neither kind of copy was ever removed. AttachedDownloads cannot carry this: it also holds the message template's attachment, which many e-mails share. InvoiceDownloadId marks the one download an e-mail owns, so deleting the e-mail can delete it safely. A requeued e-mail gets its own copy for the same reason. --- .ai/skills/message-notification.md | 5 +- .../Orders/OrderNotificationCommandHandler.cs | 23 +--- .../Orders/OrderStatusCommandHandler.cs | 16 +-- .../Orders/ProcessOrderPaidCommandHandler.cs | 20 +-- .../Services/Pdf/PdfService.cs | 22 --- .../Interfaces/Common/Pdf/IPdfService.cs | 12 -- .../Messages/IMessageProviderService.cs | 3 +- .../Messages/IOrderMessageProviderService.cs | 28 ++-- .../Messages/IQueuedEmailService.cs | 13 +- .../Interfaces/Storage/IDownloadService.cs | 6 + .../Services/MessageProviderService.cs | 3 +- .../Services/OrderMessageProviderService.cs | 75 +++++----- .../Services/QueuedEmailService.cs | 66 ++++++++- .../Services/DownloadService.cs | 14 ++ src/Core/Grand.Domain/Messages/QueuedEmail.cs | 6 + src/Core/Grand.Domain/Orders/OrderSettings.cs | 5 - .../QueuedMessagesSendScheduleTask.cs | 5 +- .../OrderNotificationCommandHandlerTests.cs | 56 +++++++- .../Orders/OrderStatusCommandHandlerTests.cs | 32 +++++ .../ProcessOrderPaidCommandHandlerTests.cs | 24 +++- .../Services/Pdf/PdfServiceTests.cs | 35 ++++- .../Services/MessageProviderServiceTest.cs | 53 ++++++- .../Services/QueuedEmailServiceTests.cs | 129 +++++++++++++++++- .../Services/DownloadServiceTests.cs | 31 +++++ ...s.OrderSettingsModel_ToDomain.verified.txt | 1 - ...ngTests.OrderSettings_ToModel.verified.txt | 1 - .../AdminShared/SystemMappingTests.cs | 11 ++ .../QueuedMessagesSendScheduleTaskTests.cs | 39 ++++++ .../Partials/Sales.TabOrderSettings.cshtml | 1 - .../Controllers/QueuedEmailController.cs | 1 + .../Models/Settings/SalesSettingsModel.cs | 3 - .../Partials/Sales.TabOrderSettings.cshtml | 1 - .../App_Data/Resources/DefaultLanguage.xml | Bin 1649146 -> 1648866 bytes 33 files changed, 567 insertions(+), 173 deletions(-) diff --git a/.ai/skills/message-notification.md b/.ai/skills/message-notification.md index 015d0a7530..1e35bdeb20 100644 --- a/.ai/skills/message-notification.md +++ b/.ai/skills/message-notification.md @@ -107,9 +107,12 @@ Task SendNotification( IEnumerable attachedDownloads = null, string replyToEmailAddress = null, string replyToName = null, string fromEmail = null, string fromName = null, string subject = null, - Reference reference = Reference.None, string objectId = ""); + Reference reference = Reference.None, string objectId = "", + string invoiceDownloadId = null); ``` +`attachedDownloads` may be shared (a message template's attachment), so deleting an email leaves them alone. `invoiceDownloadId` is owned by that one email: `QueuedEmailService` deletes it with the email, and a requeued email gets its own copy. + ### LiquidObjectBuilder (`src/Business/Grand.Business.Messages/DotLiquidDrops/LiquidObjectBuilder.cs`) ```csharp var liquidObject = await new LiquidObjectBuilder(_mediator) diff --git a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderNotificationCommandHandler.cs b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderNotificationCommandHandler.cs index f0ef78cbd4..faec880b78 100644 --- a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderNotificationCommandHandler.cs +++ b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderNotificationCommandHandler.cs @@ -65,25 +65,13 @@ await _orderMessageProviderService.SendOrderPlacedStoreOwnerMessage(request.Orde if (request.NotifyCustomer) { - string orderPlacedAttachmentFilePath = string.Empty, orderPlacedAttachmentFileName = string.Empty; - var orderPlacedAttachments = new List(); + string invoiceDownloadId = null; try { - orderPlacedAttachmentFilePath = - _orderSettings.AttachPdfInvoiceToOrderPlacedEmail && !_orderSettings.AttachPdfInvoiceToBinary - ? await _pdfService.PrintOrderToPdf(request.Order, request.Order.CustomerLanguageId) - : null; - orderPlacedAttachmentFileName = - _orderSettings.AttachPdfInvoiceToOrderPlacedEmail && !_orderSettings.AttachPdfInvoiceToBinary - ? "order.pdf" - : null; - orderPlacedAttachments = _orderSettings.AttachPdfInvoiceToOrderPlacedEmail && - _orderSettings.AttachPdfInvoiceToBinary - ? [ - await _pdfService.SaveOrderToBinary(request.Order, request.Order.CustomerLanguageId) - ] - : []; + if (_orderSettings.AttachPdfInvoiceToOrderPlacedEmail) + invoiceDownloadId = + await _pdfService.SaveOrderToBinary(request.Order, request.Order.CustomerLanguageId); } catch (Exception ex) { @@ -91,8 +79,7 @@ await _pdfService.SaveOrderToBinary(request.Order, request.Order.CustomerLanguag } await _orderMessageProviderService.SendOrderPlacedCustomerMessage(request.Order, request.Customer, - request.Order.CustomerLanguageId, orderPlacedAttachmentFilePath, orderPlacedAttachmentFileName, - orderPlacedAttachments, request.Host); + request.Order.CustomerLanguageId, invoiceDownloadId, request.Host); } if (request.NotifyStoreOwnerAndVendors && request.Order.OrderItems.Any(x => !string.IsNullOrEmpty(x.VendorId))) diff --git a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs index 18dc73197a..db5c785ccb 100644 --- a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs +++ b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs @@ -66,23 +66,13 @@ await _orderService.InsertOrderNote(new OrderNote { { //notification - var orderCompletedAttachmentFilePath = _orderSettings.AttachPdfInvoiceToOrderCompletedEmail - ? await _pdfService.PrintOrderToPdf(request.Order, "") + var invoiceDownloadId = _orderSettings.AttachPdfInvoiceToOrderCompletedEmail + ? await _pdfService.SaveOrderToBinary(request.Order, "") : null; - var orderCompletedAttachmentFileName = - _orderSettings.AttachPdfInvoiceToOrderCompletedEmail ? "order.pdf" : null; - - var orderCompletedAttachments = _orderSettings.AttachPdfInvoiceToOrderCompletedEmail && - _orderSettings.AttachPdfInvoiceToBinary - ? [ - await _pdfService.SaveOrderToBinary(request.Order, "") - ] - : new List(); await _orderMessageProviderService .SendOrderCompletedCustomerMessage(request.Order, customer, request.Order.CustomerLanguageId, - orderCompletedAttachmentFilePath, - orderCompletedAttachmentFileName, orderCompletedAttachments); + invoiceDownloadId); } if (prevOrderStatus != (int)OrderStatusSystem.Cancelled && diff --git a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs index 0f0b2c97a3..61077b01a6 100644 --- a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs +++ b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs @@ -61,24 +61,12 @@ protected virtual async Task ProcessOrderPaid(Order order) { var customer = await _customerService.GetCustomerById(order.CustomerId); - var orderPaidAttachmentFilePath = - _orderSettings.AttachPdfInvoiceToOrderPaidEmail && !_orderSettings.AttachPdfInvoiceToBinary - ? await _pdfService.PrintOrderToPdf(order, "") - : null; - var orderPaidAttachmentFileName = - _orderSettings.AttachPdfInvoiceToOrderPaidEmail && !_orderSettings.AttachPdfInvoiceToBinary - ? "order.pdf" - : null; - - var orderPaidAttachments = - _orderSettings.AttachPdfInvoiceToOrderPaidEmail && _orderSettings.AttachPdfInvoiceToBinary - ? [ - await _pdfService.SaveOrderToBinary(order, "") - ] - : new List(); + var invoiceDownloadId = _orderSettings.AttachPdfInvoiceToOrderPaidEmail + ? await _pdfService.SaveOrderToBinary(order, "") + : null; await _orderMessageProviderService.SendOrderPaidCustomerMessage(order, customer, order.CustomerLanguageId, - orderPaidAttachmentFilePath, orderPaidAttachmentFileName, orderPaidAttachments); + invoiceDownloadId); await _orderMessageProviderService.SendOrderPaidStoreOwnerMessage(order, customer, _languageSettings.DefaultAdminLanguageId); diff --git a/src/Business/Grand.Business.Common/Services/Pdf/PdfService.cs b/src/Business/Grand.Business.Common/Services/Pdf/PdfService.cs index c2bab63c79..88e8305ade 100644 --- a/src/Business/Grand.Business.Common/Services/Pdf/PdfService.cs +++ b/src/Business/Grand.Business.Common/Services/Pdf/PdfService.cs @@ -17,9 +17,7 @@ using Grand.Domain.Tax; using Grand.Mediator; using Grand.SharedKernel.Extensions; -using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Logging; -using Path = System.IO.Path; namespace Grand.Business.Common.Services.Pdf; @@ -44,13 +42,11 @@ public class PdfService : IPdfService private readonly IStoreFilesContext _storeFilesContext; private readonly IStoreService _storeService; private readonly ITranslationService _translationService; - private readonly IWebHostEnvironment _webHostEnvironment; public PdfService( IRepository downloadRepository, ITranslationService translationService, IStoreFilesContext storeFilesContext, - IWebHostEnvironment webHostEnvironment, IProductService productService, ICountryService countryService, IStoreService storeService, @@ -65,7 +61,6 @@ public PdfService( _downloadRepository = downloadRepository; _translationService = translationService; _storeFilesContext = storeFilesContext; - _webHostEnvironment = webHostEnvironment; _productService = productService; _countryService = countryService; _storeService = storeService; @@ -97,23 +92,6 @@ public async Task PrintOrdersToPdf(Stream stream, IList orders, string la await stream.WriteAsync(pdf.Build(), cancellationToken); } - public async Task PrintOrderToPdf(Order order, string languageId, string vendorId = "") - { - ArgumentNullException.ThrowIfNull(order); - - var fileName = $"order_{order.OrderGuid}_{CommonHelper.GenerateRandomDigitCode(4)}.pdf"; - - var dir = Path.Combine(_webHostEnvironment.WebRootPath, "assets/files/exportimport"); - ArgumentNullException.ThrowIfNull(dir); - - if (!System.IO.Directory.Exists(dir)) System.IO.Directory.CreateDirectory(dir); - - var filePath = Path.Combine(dir, fileName); - await using var fileStream = new FileStream(filePath, FileMode.Create); - await PrintOrdersToPdf(fileStream, [order], languageId, vendorId); - return filePath; - } - public async Task PrintPackagingSlipsToPdf(Stream stream, IList shipments, string languageId = "", CancellationToken cancellationToken = default) { diff --git a/src/Business/Grand.Business.Core/Interfaces/Common/Pdf/IPdfService.cs b/src/Business/Grand.Business.Core/Interfaces/Common/Pdf/IPdfService.cs index c198ddf5d9..9ebc13c558 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Common/Pdf/IPdfService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Common/Pdf/IPdfService.cs @@ -8,18 +8,6 @@ namespace Grand.Business.Core.Interfaces.Common.Pdf; /// public interface IPdfService { - /// - /// Print an order to PDF - /// - /// Order - /// Language identifier; 0 to use a language used when placing an order - /// - /// Vendor identifier to limit products; 0 to to print all products. If specified, then totals won't - /// be printed - /// - /// A path of generated file - Task PrintOrderToPdf(Order order, string languageId, string vendorId = ""); - /// /// Save an order PDF /// diff --git a/src/Business/Grand.Business.Core/Interfaces/Messages/IMessageProviderService.cs b/src/Business/Grand.Business.Core/Interfaces/Messages/IMessageProviderService.cs index 6f12440117..815b12cf82 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Messages/IMessageProviderService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Messages/IMessageProviderService.cs @@ -24,6 +24,7 @@ public interface IMessageProviderService /// Subject. If specified, then it overrides subject of a message template /// Reference object /// Reference object ident + /// Download of the invoice; the queued email owns it and deletes it with itself /// Queued email identifier Task SendNotification(MessageTemplate messageTemplate, EmailAccount emailAccount, string languageId, LiquidObject liquidObject, @@ -32,5 +33,5 @@ Task SendNotification(MessageTemplate messageTemplate, IEnumerable attachedDownloads = null, string replyToEmailAddress = null, string replyToName = null, string fromEmail = null, string fromName = null, string subject = null, - Reference reference = Reference.None, string objectId = ""); + Reference reference = Reference.None, string objectId = "", string invoiceDownloadId = null); } diff --git a/src/Business/Grand.Business.Core/Interfaces/Messages/IOrderMessageProviderService.cs b/src/Business/Grand.Business.Core/Interfaces/Messages/IOrderMessageProviderService.cs index 5734434ed9..2b3cf5c3b7 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Messages/IOrderMessageProviderService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Messages/IOrderMessageProviderService.cs @@ -50,15 +50,12 @@ Task SendOrderPlacedStoreOwnerMessage(Order order, Customer customer, strin /// Order instance /// Customer instance /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments ident /// Queued email identifier Task SendOrderPaidCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null); + string invoiceDownloadId = null); /// /// Sends an order paid notification to a vendor @@ -75,19 +72,15 @@ Task SendOrderPaidCustomerMessage(Order order, Customer customer, string la /// Order instance /// Customer /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments ident /// Queued email identifier /// /// Host of the storefront request; links use it when it is one of the store's domains, else the primary domain /// Task SendOrderPlacedCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null, - DomainHost host = null); + string invoiceDownloadId = null, DomainHost host = null); /// /// Sends a shipment sent notification to a customer @@ -111,15 +104,12 @@ Task SendOrderPlacedCustomerMessage(Order order, Customer customer, string /// Order instance /// Customer instance /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments ident /// Queued email identifier Task SendOrderCompletedCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null); + string invoiceDownloadId = null); /// /// Sends an order cancelled notification to a customer diff --git a/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs b/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs index 1e0f1f0542..f3bbe6870d 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs @@ -18,13 +18,20 @@ public interface IQueuedEmailService Task UpdateQueuedEmail(QueuedEmail queuedEmail); /// - /// Deleted a queued email + /// Deleted a queued email and its invoice /// /// Queued email Task DeleteQueuedEmail(QueuedEmail queuedEmail); /// - /// Deleted a customer emails + /// Copies the invoice of a queued email, so that an email created from it owns a download of its own + /// + /// Download identifier of the invoice + /// Download identifier of the copy; null when there is no invoice to copy + Task CopyInvoiceDownload(string invoiceDownloadId); + + /// + /// Deleted a customer emails and their invoices /// /// email Task DeleteCustomerEmail(string email); @@ -67,7 +74,7 @@ Task> SearchEmails(string fromEmail, int pageIndex = 0, int pageSize = int.MaxValue); /// - /// Delete all queued emails + /// Delete all queued emails and their invoices /// Task DeleteAllEmails(); } \ No newline at end of file diff --git a/src/Business/Grand.Business.Core/Interfaces/Storage/IDownloadService.cs b/src/Business/Grand.Business.Core/Interfaces/Storage/IDownloadService.cs index 5e6465edf9..dba3222bd6 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Storage/IDownloadService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Storage/IDownloadService.cs @@ -38,4 +38,10 @@ public interface IDownloadService /// /// Download Task DeleteDownload(Download download); + + /// + /// Deletes a download by identifier, without loading its file. Does nothing when it no longer exists + /// + /// Download identifier + Task DeleteDownloadById(string downloadId); } \ No newline at end of file diff --git a/src/Business/Grand.Business.Messages/Services/MessageProviderService.cs b/src/Business/Grand.Business.Messages/Services/MessageProviderService.cs index c707833e1f..85b9093c70 100644 --- a/src/Business/Grand.Business.Messages/Services/MessageProviderService.cs +++ b/src/Business/Grand.Business.Messages/Services/MessageProviderService.cs @@ -33,7 +33,7 @@ public virtual async Task SendNotification(MessageTemplate messageTemplate, IEnumerable attachedDownloads = null, string replyToEmailAddress = null, string replyToName = null, string fromEmail = null, string fromName = null, string subject = null, - Reference reference = Reference.None, string objectId = "") + Reference reference = Reference.None, string objectId = "", string invoiceDownloadId = null) { if (string.IsNullOrEmpty(toEmailAddress)) return 0; @@ -74,6 +74,7 @@ public virtual async Task SendNotification(MessageTemplate messageTemplate, email.AttachmentFilePath = attachmentFilePath; email.AttachmentFileName = attachmentFileName; email.AttachedDownloads = attachments; + email.InvoiceDownloadId = invoiceDownloadId; email.EmailAccountId = emailAccount.Id; email.DontSendBeforeDateUtc = !messageTemplate.DelayBeforeSend.HasValue ? null diff --git a/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs b/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs index d8d3db74bf..358a4cfa1e 100644 --- a/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs +++ b/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs @@ -1,5 +1,6 @@ using Grand.Business.Core.Extensions; using Grand.Business.Core.Interfaces.Messages; +using Grand.Business.Core.Interfaces.Storage; using Grand.Business.Core.Queries.Messages; using Grand.Business.Messages.DotLiquidDrops; using Grand.Domain.Common; @@ -20,10 +21,12 @@ public class OrderMessageProviderService : IOrderMessageProviderService public OrderMessageProviderService( IMessageLookupService messageLookupService, IMessageProviderService messageProviderService, + IDownloadService downloadService, IMediator mediator) { _messageLookupService = messageLookupService; _messageProviderService = messageProviderService; + _downloadService = downloadService; _mediator = mediator; } @@ -33,6 +36,7 @@ public OrderMessageProviderService( private readonly IMessageLookupService _messageLookupService; private readonly IMessageProviderService _messageProviderService; + private readonly IDownloadService _downloadService; private readonly IMediator _mediator; #endregion @@ -143,21 +147,17 @@ private async Task SendOrderStoreOwnerMessage(string template, Order order, /// Order instance /// /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments /// /// Host of the storefront request; links use it when it is one of the store's domains, else the primary domain /// public virtual async Task SendOrderPlacedCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null, - DomainHost host = null) + string invoiceDownloadId = null, DomainHost host = null) { - return await SendOrderCustomerMessage(MessageTemplateNames.SendOrderPlacedCustomerMessage, order, customer, - languageId, attachmentFilePath, attachmentFileName, attachments, host: host); + return await SendOrderInvoiceMessage(MessageTemplateNames.SendOrderPlacedCustomerMessage, order, customer, + languageId, invoiceDownloadId, host); } /// @@ -166,17 +166,14 @@ public virtual async Task SendOrderPlacedCustomerMessage(Order order, Custo /// Order instance /// Customer instance /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments ident public virtual async Task SendOrderPaidCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null) + string invoiceDownloadId = null) { - return await SendOrderCustomerMessage(MessageTemplateNames.SendOrderPaidCustomerMessage, order, customer, - languageId, attachmentFilePath, attachmentFileName, attachments); + return await SendOrderInvoiceMessage(MessageTemplateNames.SendOrderPaidCustomerMessage, order, customer, + languageId, invoiceDownloadId); } /// @@ -185,17 +182,14 @@ public virtual async Task SendOrderPaidCustomerMessage(Order order, Custome /// Order instance /// Customer instance /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. + /// + /// Download of the invoice to attach. The queued email takes ownership of it; it is deleted when no email is queued /// - /// Attachments ident public virtual async Task SendOrderCompletedCustomerMessage(Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null) + string invoiceDownloadId = null) { - return await SendOrderCustomerMessage(MessageTemplateNames.SendOrderCompletedCustomerMessage, order, customer, - languageId, attachmentFilePath, attachmentFileName, attachments); + return await SendOrderInvoiceMessage(MessageTemplateNames.SendOrderCompletedCustomerMessage, order, customer, + languageId, invoiceDownloadId); } /// @@ -225,6 +219,22 @@ public virtual async Task SendOrderRefundedCustomerMessage(Order order, dou languageId, refundedAmount: refundedAmount); } + /// + /// Sends an order notification with an invoice to a customer + /// + private async Task SendOrderInvoiceMessage(string message, Order order, Customer customer, string languageId, + string invoiceDownloadId, DomainHost host = null) + { + var queuedEmails = await SendOrderCustomerMessage(message, order, customer, languageId, invoiceDownloadId, + host: host); + + //inactive template or no recipient - no email owns the invoice, so nothing would ever delete it + if (queuedEmails == 0) + await _downloadService.DeleteDownloadById(invoiceDownloadId); + + return queuedEmails; + } + /// /// Sends an order notification to a customer /// @@ -232,19 +242,13 @@ public virtual async Task SendOrderRefundedCustomerMessage(Order order, dou /// Order instance /// Customer instance /// Message language identifier - /// Attachment file path - /// - /// Attachment file name. If specified, then this file name will be sent to a recipient. - /// Otherwise, "AttachmentFilePath" name will be used. - /// - /// Attachments ident + /// Download of the invoice to attach /// /// /// Host of the storefront request; links use it when it is one of the store's domains, else the primary domain /// private async Task SendOrderCustomerMessage(string message, Order order, Customer customer, string languageId, - string attachmentFilePath = null, string attachmentFileName = null, IEnumerable attachments = null, - double refundedAmount = 0, DomainHost host = null) + string invoiceDownloadId = null, double refundedAmount = 0, DomainHost host = null) { ArgumentNullException.ThrowIfNull(order); @@ -273,10 +277,7 @@ private async Task SendOrderCustomerMessage(string message, Order order, Cu return await _messageProviderService.SendNotification(messageTemplate, emailAccount, languageId, liquidObject, toEmail, toName, - attachmentFilePath, - attachmentFileName, - attachments, - reference: Reference.Order, objectId: order.Id); + reference: Reference.Order, objectId: order.Id, invoiceDownloadId: invoiceDownloadId); } /// diff --git a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs index 99bc1a2e2c..626908c575 100644 --- a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs +++ b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs @@ -1,15 +1,20 @@ using Grand.Business.Core.Interfaces.Messages; +using Grand.Business.Core.Interfaces.Storage; using Grand.Data; using Grand.Domain; using Grand.Domain.Common; +using Grand.Domain.Media; using Grand.Domain.Messages; using Grand.Infrastructure.Extensions; using Grand.Mediator; +using Microsoft.Extensions.Logging; namespace Grand.Business.Messages.Services; public class QueuedEmailService : IQueuedEmailService { + private readonly IDownloadService _downloadService; + private readonly ILogger _logger; private readonly IMediator _mediator; private readonly IRepository _queuedEmailRepository; @@ -17,12 +22,18 @@ public class QueuedEmailService : IQueuedEmailService /// Ctor /// /// Queued email repository + /// Download service /// Mediator + /// Logger public QueuedEmailService(IRepository queuedEmailRepository, - IMediator mediator) + IDownloadService downloadService, + IMediator mediator, + ILogger logger) { _queuedEmailRepository = queuedEmailRepository; + _downloadService = downloadService; _mediator = mediator; + _logger = logger; } /// @@ -54,7 +65,7 @@ public virtual async Task UpdateQueuedEmail(QueuedEmail queuedEmail) } /// - /// Deleted a queued email + /// Deleted a queued email and its invoice /// /// Queued email public virtual async Task DeleteQueuedEmail(QueuedEmail queuedEmail) @@ -63,12 +74,36 @@ public virtual async Task DeleteQueuedEmail(QueuedEmail queuedEmail) await _queuedEmailRepository.DeleteAsync(queuedEmail); + await DeleteInvoices([queuedEmail.InvoiceDownloadId]); + //event notification await _mediator.EntityDeleted(queuedEmail); } /// - /// Deleted a customer emails + /// Copies the invoice of a queued email, so that an email created from it owns a download of its own + /// + /// Download identifier of the invoice + /// Download identifier of the copy; null when there is no invoice to copy + public virtual async Task CopyInvoiceDownload(string invoiceDownloadId) + { + var invoice = await _downloadService.GetDownloadById(invoiceDownloadId); + if (invoice == null) + return null; + + var copy = new Download { + Filename = invoice.Filename, + Extension = invoice.Extension, + ContentType = invoice.ContentType, + DownloadBinary = invoice.DownloadBinary + }; + await _downloadService.InsertDownload(copy); + + return copy.Id; + } + + /// + /// Deleted a customer emails and their invoices /// /// email public virtual async Task DeleteCustomerEmail(string email) @@ -76,8 +111,12 @@ public virtual async Task DeleteCustomerEmail(string email) ArgumentNullException.ThrowIfNull(email); var deleteCustomerEmail = _queuedEmailRepository.Table.Where(x => x.To == email); + var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( + deleteCustomerEmail.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); await _queuedEmailRepository.DeleteAsync(deleteCustomerEmail); + + await DeleteInvoices(invoiceDownloadIds); } /// @@ -177,10 +216,29 @@ public virtual async Task> SearchEmails(string fromEmail } /// - /// Delete all queued emails + /// Delete all queued emails and their invoices /// public virtual async Task DeleteAllEmails() { + var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( + _queuedEmailRepository.Table.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); + await _queuedEmailRepository.ClearAsync(); + + await DeleteInvoices(invoiceDownloadIds); + } + + private async Task DeleteInvoices(IEnumerable invoiceDownloadIds) + { + foreach (var invoiceDownloadId in invoiceDownloadIds) + try + { + await _downloadService.DeleteDownloadById(invoiceDownloadId); + } + catch (Exception ex) + { + //the emails are gone already; a broken file must not stop the rest nor the deletion of an account + _logger.LogError(ex, "Error deleting the invoice download {InvoiceDownloadId}", invoiceDownloadId); + } } } \ No newline at end of file diff --git a/src/Business/Grand.Business.Storage/Services/DownloadService.cs b/src/Business/Grand.Business.Storage/Services/DownloadService.cs index 9cdde91329..c5327ebc80 100644 --- a/src/Business/Grand.Business.Storage/Services/DownloadService.cs +++ b/src/Business/Grand.Business.Storage/Services/DownloadService.cs @@ -131,5 +131,19 @@ public virtual async Task DeleteDownload(Download download) await _mediator.EntityDeleted(download); } + /// + /// Deletes a download by identifier, without loading its file. Does nothing when it no longer exists + /// + /// Download identifier + public virtual async Task DeleteDownloadById(string downloadId) + { + if (string.IsNullOrEmpty(downloadId)) + return; + + var download = await _downloadRepository.GetByIdAsync(downloadId); + if (download != null) + await DeleteDownload(download); + } + #endregion } \ No newline at end of file diff --git a/src/Core/Grand.Domain/Messages/QueuedEmail.cs b/src/Core/Grand.Domain/Messages/QueuedEmail.cs index 011eef4d89..3f8600136a 100644 --- a/src/Core/Grand.Domain/Messages/QueuedEmail.cs +++ b/src/Core/Grand.Domain/Messages/QueuedEmail.cs @@ -78,6 +78,12 @@ public class QueuedEmail : BaseEntity /// public IList AttachedDownloads { get; set; } = new List(); + /// + /// Gets or sets the download identifier of the order invoice generated for this email. + /// Unlike AttachedDownloads, the email owns this download and it is deleted together with the email + /// + public string InvoiceDownloadId { get; set; } + /// /// Gets or sets the date and time in UTC before which this email should not be sent /// diff --git a/src/Core/Grand.Domain/Orders/OrderSettings.cs b/src/Core/Grand.Domain/Orders/OrderSettings.cs index 0dcc6e31ee..1581cdd020 100644 --- a/src/Core/Grand.Domain/Orders/OrderSettings.cs +++ b/src/Core/Grand.Domain/Orders/OrderSettings.cs @@ -60,11 +60,6 @@ public class OrderSettings : ISettings /// public bool AttachPdfInvoiceToOrderCompletedEmail { get; set; } - /// - /// Gets or sets a value indicating we should attach PDF invoice to binary field - /// - public bool AttachPdfInvoiceToBinary { get; set; } - /// /// Gets or sets a value indicating whether "Merchandise returns" are allowed /// diff --git a/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/QueuedMessagesSendScheduleTask.cs b/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/QueuedMessagesSendScheduleTask.cs index f9c8648565..ad0b3c1de0 100644 --- a/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/QueuedMessagesSendScheduleTask.cs +++ b/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/QueuedMessagesSendScheduleTask.cs @@ -43,6 +43,9 @@ public async Task Execute(CancellationToken cancellationToken) var cc = string.IsNullOrWhiteSpace(queuedEmail.CC) ? null : queuedEmail.CC.Split([';'], StringSplitOptions.RemoveEmptyEntries); + var attachedDownloads = string.IsNullOrEmpty(queuedEmail.InvoiceDownloadId) + ? queuedEmail.AttachedDownloads + : queuedEmail.AttachedDownloads.Append(queuedEmail.InvoiceDownloadId); try { @@ -60,7 +63,7 @@ await _emailSender.SendEmail(emailAccount, cc, queuedEmail.AttachmentFilePath, queuedEmail.AttachmentFileName, - queuedEmail.AttachedDownloads, + attachedDownloads, cancellationToken); queuedEmail.SentOnUtc = DateTime.UtcNow; diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderNotificationCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderNotificationCommandHandlerTests.cs index 369f6c1d33..12608a9d20 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderNotificationCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderNotificationCommandHandlerTests.cs @@ -21,14 +21,18 @@ public class OrderNotificationCommandHandlerTests private OrderNotificationCommandHandler _handler; private Mock _orderMessageProviderServiceMock; private Mock _orderServiceMock; + private OrderSettings _orderSettings; + private Mock _pdfServiceMock; [TestInitialize] public void Init() { _orderServiceMock = new Mock(); _orderMessageProviderServiceMock = new Mock(); + _orderSettings = new OrderSettings(); + _pdfServiceMock = new Mock(); _handler = new OrderNotificationCommandHandler(_orderServiceMock.Object, _orderMessageProviderServiceMock.Object, - new OrderSettings(), new LanguageSettings(), new Mock().Object, + _orderSettings, new LanguageSettings(), _pdfServiceMock.Object, new Mock>().Object, new Mock().Object); } @@ -101,7 +105,7 @@ await _handler.Handle(new OrderNotificationCommand { _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedVendorMessage(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(order, customer, "en", - It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny()), Times.Once); + It.IsAny(), It.IsAny()), Times.Once); } [TestMethod] @@ -115,8 +119,7 @@ await _handler.Handle(new OrderNotificationCommand { }, CancellationToken.None); _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny>(), It.IsAny()), Times.Never); + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedStoreOwnerMessage(order, customer, It.IsAny(), It.IsAny()), Times.Once); } @@ -133,6 +136,49 @@ public async Task Handle_SendsTheCustomerMessageForTheCommandCustomer() //Assert _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(order, customer, "en", - It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); + It.IsAny(), It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Handle_InvoiceEnabled_AttachesTheSavedInvoice() + { + _orderSettings.AttachPdfInvoiceToOrderPlacedEmail = true; + var customer = new Customer(); + var order = new Order { Id = "order", CustomerLanguageId = "en" }; + _pdfServiceMock.Setup(x => x.SaveOrderToBinary(order, "en", It.IsAny())).ReturnsAsync("invoice"); + + await _handler.Handle(new OrderNotificationCommand { Order = order, Customer = customer }, CancellationToken.None); + + _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(order, customer, "en", "invoice", + It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Handle_InvoiceDisabled_SavesNoInvoice() + { + var customer = new Customer(); + var order = new Order { Id = "order", CustomerLanguageId = "en" }; + + await _handler.Handle(new OrderNotificationCommand { Order = order, Customer = customer }, CancellationToken.None); + + _pdfServiceMock.Verify(x => x.SaveOrderToBinary(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(order, customer, "en", null, + It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Handle_InvoiceCannotBeGenerated_StillSendsTheCustomerMessage() + { + _orderSettings.AttachPdfInvoiceToOrderPlacedEmail = true; + var customer = new Customer(); + var order = new Order { Id = "order", CustomerLanguageId = "en" }; + _pdfServiceMock.Setup(x => x.SaveOrderToBinary(order, "en", It.IsAny())) + .ThrowsAsync(new InvalidOperationException()); + + await _handler.Handle(new OrderNotificationCommand { Order = order, Customer = customer }, CancellationToken.None); + + _orderMessageProviderServiceMock.Verify(x => x.SendOrderPlacedCustomerMessage(order, customer, "en", null, + It.IsAny()), Times.Once); } } diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs index a633ae22ab..e90ea4f80a 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs @@ -4,6 +4,7 @@ using Grand.Business.Core.Interfaces.Common.Pdf; using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Messages; +using Grand.Domain.Customers; using Grand.Domain.Orders; using Grand.Mediator; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -52,4 +53,35 @@ public async Task HandleTest() //Assert _orderServiceMock.Verify(c => c.UpdateOrder(It.IsAny()), Times.Once); } + + [TestMethod] + public async Task Handle_OrderCompleted_InvoiceEnabled_AttachesTheSavedInvoice() + { + _orderSettings.AttachPdfInvoiceToOrderCompletedEmail = true; + var order = new Order { OrderStatusId = (int)OrderStatusSystem.Pending, CustomerLanguageId = "en" }; + _pdfServiceMock.Setup(c => c.SaveOrderToBinary(order, It.IsAny(), It.IsAny())) + .ReturnsAsync("invoice"); + + await _handler.Handle( + new SetOrderStatusCommand { Order = order, Os = OrderStatusSystem.Complete, NotifyCustomer = true }, + CancellationToken.None); + + _orderMessageProviderServiceMock.Verify(c => c.SendOrderCompletedCustomerMessage(order, It.IsAny(), + "en", "invoice"), Times.Once); + } + + [TestMethod] + public async Task Handle_OrderCompleted_InvoiceDisabled_SavesNoInvoice() + { + var order = new Order { OrderStatusId = (int)OrderStatusSystem.Pending, CustomerLanguageId = "en" }; + + await _handler.Handle( + new SetOrderStatusCommand { Order = order, Os = OrderStatusSystem.Complete, NotifyCustomer = true }, + CancellationToken.None); + + _pdfServiceMock.Verify(c => c.SaveOrderToBinary(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + _orderMessageProviderServiceMock.Verify(c => c.SendOrderCompletedCustomerMessage(order, It.IsAny(), + "en", null), Times.Once); + } } \ No newline at end of file diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs index 2b0a0708a4..686026b93b 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs @@ -20,6 +20,8 @@ public class ProcessOrderPaidCommandHandlerTests private ProcessOrderPaidCommandHandler _handler; private Mock _mediatorMock; private Mock _messageProviderMock; + private OrderSettings _orderSettings; + private Mock _pdfServiceMock; [TestInitialize] public void Init() @@ -28,8 +30,10 @@ public void Init() _customerServiceMock.Setup(x => x.GetCustomerById(It.IsAny())).ReturnsAsync(new Customer()); _mediatorMock = new Mock(); _messageProviderMock = new Mock(); + _pdfServiceMock = new Mock(); + _orderSettings = new OrderSettings(); _handler = new ProcessOrderPaidCommandHandler(_messageProviderMock.Object, _customerServiceMock.Object, - _mediatorMock.Object, new Mock().Object, new OrderSettings(), new LanguageSettings()); + _mediatorMock.Object, _pdfServiceMock.Object, _orderSettings, new LanguageSettings()); } [TestMethod] @@ -53,8 +57,24 @@ public async Task Handle_PendingOrder_PublishesAndSendsPaidMessages() _mediatorMock.Verify(x => x.Publish(It.IsAny(), It.IsAny()), Times.Once); _messageProviderMock.Verify(x => x.SendOrderPaidCustomerMessage(order, It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); + It.IsAny(), null), Times.Once); _messageProviderMock.Verify(x => x.SendOrderPaidStoreOwnerMessage(order, It.IsAny(), It.IsAny()), Times.Once); + _pdfServiceMock.Verify(x => x.SaveOrderToBinary(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task Handle_InvoiceEnabled_AttachesTheSavedInvoice() + { + _orderSettings.AttachPdfInvoiceToOrderPaidEmail = true; + var order = new Order { OrderTotal = 10, OrderStatusId = (int)OrderStatusSystem.Pending }; + _pdfServiceMock.Setup(x => x.SaveOrderToBinary(order, It.IsAny(), It.IsAny())) + .ReturnsAsync("invoice"); + + await _handler.Handle(new ProcessOrderPaidCommand { Order = order }, CancellationToken.None); + + _messageProviderMock.Verify(x => x.SendOrderPaidCustomerMessage(order, It.IsAny(), + It.IsAny(), "invoice"), Times.Once); } } diff --git a/src/Tests/Grand.Business.Common.Tests/Services/Pdf/PdfServiceTests.cs b/src/Tests/Grand.Business.Common.Tests/Services/Pdf/PdfServiceTests.cs index 4fd5d958a6..b25cf92ceb 100644 --- a/src/Tests/Grand.Business.Common.Tests/Services/Pdf/PdfServiceTests.cs +++ b/src/Tests/Grand.Business.Common.Tests/Services/Pdf/PdfServiceTests.cs @@ -20,7 +20,6 @@ using Grand.Domain.Stores; using Grand.Domain.Tax; using Grand.Mediator; -using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -31,12 +30,14 @@ namespace Grand.Business.Common.Tests.Services.Pdf; [TestClass] public class PdfServiceTests { + private Mock> _downloadRepositoryMock; private Mock _mediatorMock; private Mock _orderServiceMock; private PdfSettings _pdfSettings; private Mock _pictureServiceMock; private Mock _productServiceMock; private PdfService _service; + private Mock _storeFilesContextMock; [TestInitialize] public void Init() @@ -64,8 +65,11 @@ public void Init() _productServiceMock.Setup(p => p.GetProductByIdIncludeArch(It.IsAny())) .ReturnsAsync((string id) => new Product { Id = id, Name = "Product " + id, Sku = "SKU-" + id }); - _service = new PdfService(new Mock>().Object, translationServiceMock.Object, - new Mock().Object, new Mock().Object, + _downloadRepositoryMock = new Mock>(); + _storeFilesContextMock = new Mock(); + + _service = new PdfService(_downloadRepositoryMock.Object, translationServiceMock.Object, + _storeFilesContextMock.Object, _productServiceMock.Object, new Mock().Object, storeServiceMock.Object, _pictureServiceMock.Object, new Mock().Object, giftVoucherServiceMock.Object, _orderServiceMock.Object, _pdfSettings, _mediatorMock.Object, @@ -306,6 +310,31 @@ public async Task PrintPackagingSlipsToPdf_WhenEveryOrderIsMissing_WritesOneEmpt Assert.AreEqual(1, pdf.NumberOfPages); } + [TestMethod] + public async Task SaveOrderToBinary_StoresThePdfAsADownloadTheStorefrontCannotServe() + { + //Arrange + byte[] uploaded = null; + _storeFilesContextMock.Setup(s => s.BucketUploadFromBytes(It.IsAny(), It.IsAny())) + .Callback((_, bytes) => uploaded = bytes) + .ReturnsAsync("object"); + Download inserted = null; + _downloadRepositoryMock.Setup(r => r.InsertAsync(It.IsAny())) + .Callback(d => inserted = d); + //Act + var downloadId = await _service.SaveOrderToBinary(CreateOrder(), ""); + //Assert + Assert.IsNotNull(inserted); + Assert.AreEqual(inserted.Id, downloadId); + Assert.AreEqual("object", inserted.DownloadObjectId); + Assert.AreEqual(".pdf", inserted.Extension); + Assert.AreEqual("application/pdf", inserted.ContentType); + //the download action rejects an empty guid, and only an owner or an admin gets past it anyway + Assert.AreEqual(Guid.Empty, inserted.DownloadGuid); + using var pdf = PdfDocument.Open(uploaded); + Assert.AreEqual(1, pdf.NumberOfPages); + } + [TestMethod] public async Task PrintOrdersToPdf_RowTallerThanAPage_IsSplitAcrossPagesAboveTheFooter() { diff --git a/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs b/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs index 58e60f68f8..17fe8d526f 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs @@ -3,6 +3,7 @@ using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Stores; using Grand.Business.Core.Interfaces.Messages; +using Grand.Business.Core.Interfaces.Storage; using Grand.Business.Core.Queries.Messages; using Grand.Business.Messages.DotLiquidDrops; using Grand.Business.Messages.Services; @@ -26,6 +27,7 @@ namespace Grand.Business.Messages.Tests.Services; public class MessageProviderServiceTest { private Mock _commonSetting; + private Mock _downloadServiceMock; private Mock _emailAccountServiceMock; private Mock _emailAccountSettingsMock; private Mock _groupService; @@ -92,8 +94,9 @@ public void Init() _mediatorMock.Object, _commonSetting.Object); _vendorMessageService = new VendorMessageProviderService(messageLookupService, _messageService, _mediatorMock.Object, _commonSetting.Object); + _downloadServiceMock = new Mock(); _orderMessageService = new OrderMessageProviderService(messageLookupService, _messageService, - _mediatorMock.Object); + _downloadServiceMock.Object, _mediatorMock.Object); _newsletterMessageService = new NewsletterMessageProviderService(messageLookupService, _messageService, _mediatorMock.Object); _merchandiseReturnMessageService = new MerchandiseReturnMessageProviderService(messageLookupService, @@ -147,6 +150,49 @@ public async Task SendOrderPaidStoreOwnerMessage_QueuesEmail() _queuedEmailServiceMock.Verify(x => x.InsertQueuedEmail(It.IsAny()), Times.Once()); } + [TestMethod] + public async Task SendOrderPaidCustomerMessage_TheEmailOwnsTheInvoiceAndKeepsTheTemplateAttachment() + { + _messageTemplateServiceMock.Setup(x => x.GetMessageTemplateByName(It.IsAny(), It.IsAny())) + .ReturnsAsync(new MessageTemplate { IsActive = true, AttachedDownloadId = "template-attachment" }); + QueuedEmail queuedEmail = null; + _queuedEmailServiceMock.Setup(x => x.InsertQueuedEmail(It.IsAny())) + .Callback(email => queuedEmail = email); + + var result = await _orderMessageService.SendOrderPaidCustomerMessage(OrderWithBillingEmail("a@b.c"), + new Customer(), "123", "invoice"); + + Assert.AreEqual(1, result); + Assert.AreEqual("invoice", queuedEmail.InvoiceDownloadId); + CollectionAssert.AreEqual(new[] { "template-attachment" }, queuedEmail.AttachedDownloads.ToArray()); + Assert.IsNull(queuedEmail.AttachmentFilePath); + _downloadServiceMock.Verify(x => x.DeleteDownloadById(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task SendOrderPlacedCustomerMessage_InactiveTemplate_DeletesTheInvoice() + { + _messageTemplateServiceMock.Setup(x => x.GetMessageTemplateByName(It.IsAny(), It.IsAny())) + .ReturnsAsync(new MessageTemplate { IsActive = false }); + + var result = await _orderMessageService.SendOrderPlacedCustomerMessage(OrderWithBillingEmail("a@b.c"), + new Customer(), "123", "invoice"); + + Assert.AreEqual(0, result); + _downloadServiceMock.Verify(x => x.DeleteDownloadById("invoice"), Times.Once); + } + + [TestMethod] + public async Task SendOrderCompletedCustomerMessage_NoRecipient_DeletesTheInvoice() + { + var result = await _orderMessageService.SendOrderCompletedCustomerMessage(OrderWithBillingEmail(null), + new Customer(), "123", "invoice"); + + Assert.AreEqual(0, result); + _queuedEmailServiceMock.Verify(x => x.InsertQueuedEmail(It.IsAny()), Times.Never); + _downloadServiceMock.Verify(x => x.DeleteDownloadById("invoice"), Times.Once); + } + [TestMethod] public async Task SendNewsLetterSubscriptionActivationMessage_QueuesEmail() { @@ -200,6 +246,11 @@ public async Task SendCustomerMessage_StoreWithoutDomains_LinksPointToTheStoreUr StringAssert.StartsWith(url, "https://store.example/"); } + private static Order OrderWithBillingEmail(string email) + { + return new Order { BillingAddress = new Address { Email = email } }; + } + private async Task PasswordRecoveryUrl(Store store) { LiquidCustomer liquidCustomer = null; diff --git a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs index d4478561e2..b7ab8f6936 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs @@ -1,8 +1,11 @@ -using Grand.Business.Messages.Services; +using Grand.Business.Core.Interfaces.Storage; +using Grand.Business.Messages.Services; using Grand.Data; +using Grand.Domain.Media; using Grand.Domain.Messages; using Grand.Infrastructure.Events; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -11,6 +14,8 @@ namespace Grand.Business.Messages.Tests.Services; [TestClass] public class QueuedEmailServiceTests { + private Mock _downloadServiceMock; + private Mock> _loggerMock; private Mock _mediatorMock; private Mock> _repository; private QueuedEmailService _service; @@ -19,8 +24,13 @@ public class QueuedEmailServiceTests public void Init() { _repository = new Mock>(); + _repository.Setup(c => c.ToListAsync(It.IsAny>(), It.IsAny())) + .Returns((IQueryable query, CancellationToken _) => Task.FromResult>(query.ToList())); + _downloadServiceMock = new Mock(); _mediatorMock = new Mock(); - _service = new QueuedEmailService(_repository.Object, _mediatorMock.Object); + _loggerMock = new Mock>(); + _service = new QueuedEmailService(_repository.Object, _downloadServiceMock.Object, _mediatorMock.Object, + _loggerMock.Object); } [TestMethod] @@ -88,4 +98,119 @@ public void DeleteQueuedEmai_NullArguemnt_ThrowException() Assert.ThrowsExactlyAsync(async () => await _service.DeleteQueuedEmail(new QueuedEmail())); } + + [TestMethod] + public async Task DeleteQueuedEmail_DeletesTheInvoiceButNotTheSharedAttachments() + { + await _service.DeleteQueuedEmail(new QueuedEmail { + InvoiceDownloadId = "invoice", AttachedDownloads = ["template-attachment"] + }); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById("invoice"), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById("template-attachment"), Times.Never); + } + + [TestMethod] + public async Task DeleteQueuedEmail_InvoiceCannotBeDeleted_LogsAndStillPublishesTheEvent() + { + _downloadServiceMock.Setup(c => c.DeleteDownloadById("invoice")).ThrowsAsync(new InvalidOperationException()); + + await _service.DeleteQueuedEmail(new QueuedEmail { InvoiceDownloadId = "invoice" }); + + _repository.Verify(c => c.DeleteAsync(It.IsAny()), Times.Once); + _mediatorMock.Verify(c => c.Publish(It.IsAny>(), default), Times.Once); + VerifyErrorLogged(Times.Once()); + } + + [TestMethod] + public async Task DeleteCustomerEmail_DeletesOnlyTheInvoicesOfThatCustomer() + { + SetupEmails( + new QueuedEmail { To = "a@b.c", InvoiceDownloadId = "mine" }, + new QueuedEmail { To = "a@b.c" }, + new QueuedEmail { To = "other@b.c", InvoiceDownloadId = "other" }); + + await _service.DeleteCustomerEmail("a@b.c"); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById("mine"), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task DeleteCustomerEmail_InvoiceCannotBeDeleted_DoesNotStopTheRest() + { + SetupEmails( + new QueuedEmail { To = "a@b.c", InvoiceDownloadId = "broken" }, + new QueuedEmail { To = "a@b.c", InvoiceDownloadId = "fine" }); + _downloadServiceMock.Setup(c => c.DeleteDownloadById("broken")).ThrowsAsync(new InvalidOperationException()); + + await _service.DeleteCustomerEmail("a@b.c"); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById("fine"), Times.Once); + VerifyErrorLogged(Times.Once()); + } + + [TestMethod] + public async Task DeleteAllEmails_DeletesEveryInvoice() + { + SetupEmails( + new QueuedEmail { InvoiceDownloadId = "first", AttachedDownloads = ["template-attachment"] }, + new QueuedEmail { InvoiceDownloadId = "second" }, + new QueuedEmail()); + + await _service.DeleteAllEmails(); + + _repository.Verify(c => c.ClearAsync(), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById("first"), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById("second"), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Exactly(2)); + } + + [TestMethod] + public async Task CopyInvoiceDownload_CreatesASeparateDownload() + { + var binary = new byte[] { 1, 2, 3 }; + var invoice = new Download { + Filename = "order_1", Extension = ".pdf", ContentType = "application/pdf", DownloadBinary = binary + }; + _downloadServiceMock.Setup(c => c.GetDownloadById(invoice.Id)).ReturnsAsync(invoice); + Download copy = null; + _downloadServiceMock.Setup(c => c.InsertDownload(It.IsAny())).Callback(d => copy = d); + + var result = await _service.CopyInvoiceDownload(invoice.Id); + + Assert.IsNotNull(copy); + Assert.AreEqual(copy.Id, result); + Assert.AreNotEqual(invoice.Id, result); + Assert.AreEqual("order_1", copy.Filename); + Assert.AreEqual(".pdf", copy.Extension); + Assert.AreEqual("application/pdf", copy.ContentType); + Assert.AreSame(binary, copy.DownloadBinary); + //no guid, so the storefront download action cannot serve it + Assert.AreEqual(Guid.Empty, copy.DownloadGuid); + } + + [TestMethod] + public async Task CopyInvoiceDownload_NoInvoice_ReturnsNull() + { + var result = await _service.CopyInvoiceDownload(null); + + Assert.IsNull(result); + _downloadServiceMock.Verify(c => c.InsertDownload(It.IsAny()), Times.Never); + } + + private void SetupEmails(params QueuedEmail[] emails) + { + _repository.Setup(c => c.Table).Returns(emails.AsQueryable()); + } + + private void VerifyErrorLogged(Times times) + { + _loggerMock.Verify(c => c.Log( + LogLevel.Error, + It.IsAny(), + It.IsAny(), + It.IsAny(), + (Func)It.IsAny()), times); + } } \ No newline at end of file diff --git a/src/Tests/Grand.Business.Storage.Tests/Services/DownloadServiceTests.cs b/src/Tests/Grand.Business.Storage.Tests/Services/DownloadServiceTests.cs index a4c6e6d811..97a274ecd6 100644 --- a/src/Tests/Grand.Business.Storage.Tests/Services/DownloadServiceTests.cs +++ b/src/Tests/Grand.Business.Storage.Tests/Services/DownloadServiceTests.cs @@ -76,4 +76,35 @@ public void DeleteDownload_NullArgument_ThrowException() { Assert.ThrowsExactlyAsync(async () => await _service.DeleteDownload(null)); } + + [TestMethod] + public async Task DeleteDownloadById_DeletesTheRecordAndItsFileWithoutLoadingTheFile() + { + var download = new Download { DownloadObjectId = "object" }; + _repositoryMock.Setup(c => c.GetByIdAsync("id")).ReturnsAsync(download); + + await _service.DeleteDownloadById("id"); + + _repositoryMock.Verify(c => c.DeleteAsync(download), Times.Once); + _storeFilesContext.Verify(c => c.BucketDelete("object"), Times.Once); + _storeFilesContext.Verify(c => c.BucketDownload(It.IsAny()), Times.Never); + _mediatorMock.Verify(c => c.Publish(It.IsAny>(), default), Times.Once); + } + + [TestMethod] + public async Task DeleteDownloadById_MissingDownload_DoesNothing() + { + await _service.DeleteDownloadById("missing"); + + _repositoryMock.Verify(c => c.DeleteAsync(It.IsAny()), Times.Never); + _storeFilesContext.Verify(c => c.BucketDelete(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task DeleteDownloadById_NoIdentifier_DoesNothing() + { + await _service.DeleteDownloadById(null); + + _repositoryMock.Verify(c => c.GetByIdAsync(It.IsAny()), Times.Never); + } } \ No newline at end of file diff --git a/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettingsModel_ToDomain.verified.txt b/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettingsModel_ToDomain.verified.txt index 109a0bfd8d..2c67d3bbd7 100644 --- a/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettingsModel_ToDomain.verified.txt +++ b/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettingsModel_ToDomain.verified.txt @@ -8,7 +8,6 @@ AttachPdfInvoiceToOrderPlacedEmail: false, AttachPdfInvoiceToOrderPaidEmail: false, AttachPdfInvoiceToOrderCompletedEmail: false, - AttachPdfInvoiceToBinary: false, MerchandiseReturnsEnabled: false, MerchandiseReturns_AllowToSpecifyPickupAddress: false, MerchandiseReturns_AllowToSpecifyPickupDate: false, diff --git a/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettings_ToModel.verified.txt b/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettings_ToModel.verified.txt index a049c0e219..b2a10bd43b 100644 --- a/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettings_ToModel.verified.txt +++ b/src/Tests/Grand.Mapping.Tests/AdminShared/SettingsMappingTests.OrderSettings_ToModel.verified.txt @@ -18,7 +18,6 @@ CompleteOrderWhenDelivered: false, UserCanCancelUnpaidOrder: false, AllowCustomerToAddOrderNote: false, - AttachPdfInvoiceToBinary: false, MerchandiseReturns_PickupDateRequired: false, UnpublishAuctionProduct: false } \ No newline at end of file diff --git a/src/Tests/Grand.Mapping.Tests/AdminShared/SystemMappingTests.cs b/src/Tests/Grand.Mapping.Tests/AdminShared/SystemMappingTests.cs index 40f7b57045..7388199e32 100644 --- a/src/Tests/Grand.Mapping.Tests/AdminShared/SystemMappingTests.cs +++ b/src/Tests/Grand.Mapping.Tests/AdminShared/SystemMappingTests.cs @@ -102,6 +102,17 @@ public Task QueuedEmailModel_ToDomain() return Verify(_mapper.Map(model)); } + [TestMethod] + public void QueuedEmailModel_ToExistingEntity_KeepsTheInvoice() + { + var entity = new QueuedEmail { InvoiceDownloadId = "invoice" }; + + _mapper.Map(new QueuedEmailModel { Subject = "Changed" }, entity); + + Assert.AreEqual("Changed", entity.Subject); + Assert.AreEqual("invoice", entity.InvoiceDownloadId); + } + // ── MessageTemplate ─────────────────────────────────────────────────────── [TestMethod] diff --git a/src/Tests/Grand.Modules.Tests/Services/BackgroundService/QueuedMessagesSendScheduleTaskTests.cs b/src/Tests/Grand.Modules.Tests/Services/BackgroundService/QueuedMessagesSendScheduleTaskTests.cs index 152948bc6c..710398f0f8 100644 --- a/src/Tests/Grand.Modules.Tests/Services/BackgroundService/QueuedMessagesSendScheduleTaskTests.cs +++ b/src/Tests/Grand.Modules.Tests/Services/BackgroundService/QueuedMessagesSendScheduleTaskTests.cs @@ -127,6 +127,45 @@ public async Task Execute_CancelledMidSend_DoesNotCountTheInterruptedTry() _queuedEmailServiceMock.Verify(c => c.UpdateQueuedEmail(It.IsAny()), Times.Never); } + [TestMethod] + public async Task Execute_EmailWithInvoice_SendsItWithTheOtherAttachments() + { + var attachments = await SentAttachments(new QueuedEmail { + EmailAccountId = "id", InvoiceDownloadId = "invoice", AttachedDownloads = ["template-attachment"] + }); + + CollectionAssert.AreEqual(new[] { "template-attachment", "invoice" }, attachments); + } + + [TestMethod] + public async Task Execute_EmailWithoutInvoice_SendsOnlyTheOtherAttachments() + { + var attachments = await SentAttachments(new QueuedEmail { + EmailAccountId = "id", AttachedDownloads = ["template-attachment"] + }); + + CollectionAssert.AreEqual(new[] { "template-attachment" }, attachments); + } + + private async Task SentAttachments(QueuedEmail queuedEmail) + { + SetupQueue(queuedEmail); + string[] attachments = null; + _emailSenderMock.Setup(c => c.SendEmail(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny>(), + It.IsAny>(), It.IsAny(), It.IsAny(), + It.IsAny>(), It.IsAny())) + .Callback((EmailAccount _, string _, string _, string _, string _, string _, string _, string _, string _, + IEnumerable _, IEnumerable _, string _, string _, IEnumerable downloads, + CancellationToken _) => attachments = downloads.ToArray()) + .Returns(Task.CompletedTask); + + await _task.Execute(CancellationToken.None); + + return attachments; + } + private void SetupQueue(params QueuedEmail[] queuedEmails) { _queuedEmailServiceMock.Setup(c => c.SearchEmails(It.IsAny(), It.IsAny(), It.IsAny(), diff --git a/src/Web/Grand.Web.Admin/Areas/Admin/Views/Setting/Partials/Sales.TabOrderSettings.cshtml b/src/Web/Grand.Web.Admin/Areas/Admin/Views/Setting/Partials/Sales.TabOrderSettings.cshtml index 18e62a9caa..110b5ee3e1 100644 --- a/src/Web/Grand.Web.Admin/Areas/Admin/Views/Setting/Partials/Sales.TabOrderSettings.cshtml +++ b/src/Web/Grand.Web.Admin/Areas/Admin/Views/Setting/Partials/Sales.TabOrderSettings.cshtml @@ -33,7 +33,6 @@ - diff --git a/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs b/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs index fd9df8225c..5f30efd408 100644 --- a/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs @@ -190,6 +190,7 @@ public async Task Requeue(QueuedEmailModel queuedEmailModel) AttachmentFilePath = queuedEmail.AttachmentFilePath, AttachmentFileName = queuedEmail.AttachmentFileName, AttachedDownloads = queuedEmail.AttachedDownloads, + InvoiceDownloadId = await _queuedEmailService.CopyInvoiceDownload(queuedEmail.InvoiceDownloadId), EmailAccountId = queuedEmail.EmailAccountId, DontSendBeforeDateUtc = queuedEmailModel.SendImmediately || !queuedEmailModel.DontSendBeforeDate.HasValue ? null diff --git a/src/Web/Grand.Web.AdminShared/Models/Settings/SalesSettingsModel.cs b/src/Web/Grand.Web.AdminShared/Models/Settings/SalesSettingsModel.cs index 03f9305cca..cd354a2a4d 100644 --- a/src/Web/Grand.Web.AdminShared/Models/Settings/SalesSettingsModel.cs +++ b/src/Web/Grand.Web.AdminShared/Models/Settings/SalesSettingsModel.cs @@ -87,9 +87,6 @@ public class OrderSettingsModel : BaseModel [GrandResourceDisplayName("Admin.Settings.Order.AllowCustomerToAddOrderNote")] public bool AllowCustomerToAddOrderNote { get; set; } - [GrandResourceDisplayName("Admin.Settings.Order.AttachPdfInvoiceToBinary")] - public bool AttachPdfInvoiceToBinary { get; set; } - [GrandResourceDisplayName("Admin.Settings.Order.MerchandiseReturns_PickupDateRequired")] public bool MerchandiseReturns_PickupDateRequired { get; set; } diff --git a/src/Web/Grand.Web.Store/Areas/Store/Views/Setting/Partials/Sales.TabOrderSettings.cshtml b/src/Web/Grand.Web.Store/Areas/Store/Views/Setting/Partials/Sales.TabOrderSettings.cshtml index 18e62a9caa..110b5ee3e1 100644 --- a/src/Web/Grand.Web.Store/Areas/Store/Views/Setting/Partials/Sales.TabOrderSettings.cshtml +++ b/src/Web/Grand.Web.Store/Areas/Store/Views/Setting/Partials/Sales.TabOrderSettings.cshtml @@ -33,7 +33,6 @@ - diff --git a/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml b/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml index a9d105e274194014bd8d113ac8fa78788952cd4a..7f27d889f67f87d81220ee7077d522e970144892 100644 GIT binary patch delta 71 zcmV~$NezH700h8r-?ZGQASF7M2#)7Pc1l7LFFq7OocV7M>Q~7QQX~FZ5WG7&001rf-yG zv!32?nvre#1Ot8r?h=N41_cI))I%5c)aec8{BqO94EXh?&oJW`YG={s2Vwyr76f7t WUl@o*fLIiW#ei6RJBz-=y`KP3!Y3{O From 8efd32084ad3d6b1d723853754db60166793c303 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 18:53:02 +0100 Subject: [PATCH 2/6] Keep processing an order when its invoice cannot be generated --- .../Orders/OrderStatusCommandHandler.cs | 20 +++++++++++++---- .../Orders/ProcessOrderPaidCommandHandler.cs | 20 +++++++++++++---- .../Orders/MarkAsPaidCommandHandlerTests.cs | 4 +++- .../Orders/OrderStatusCommandHandlerTests.cs | 22 ++++++++++++++++++- .../ProcessOrderPaidCommandHandlerTests.cs | 20 ++++++++++++++++- 5 files changed, 75 insertions(+), 11 deletions(-) diff --git a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs index db5c785ccb..0c85c3e368 100644 --- a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs +++ b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/OrderStatusCommandHandler.cs @@ -5,12 +5,14 @@ using Grand.Business.Core.Interfaces.Messages; using Grand.Domain.Orders; using Grand.Mediator; +using Microsoft.Extensions.Logging; namespace Grand.Business.Checkout.Commands.Handlers.Orders; public class OrderStatusCommandHandler : IRequestHandler { private readonly ICustomerService _customerService; + private readonly ILogger _logger; private readonly LoyaltyPointsSettings _loyaltyPointsSettings; private readonly IMediator _mediator; private readonly IOrderMessageProviderService _orderMessageProviderService; @@ -27,7 +29,8 @@ public OrderStatusCommandHandler( IVendorService vendorService, IMediator mediator, OrderSettings orderSettings, - LoyaltyPointsSettings loyaltyPointsSettings) + LoyaltyPointsSettings loyaltyPointsSettings, + ILogger logger) { _orderService = orderService; _customerService = customerService; @@ -37,6 +40,7 @@ public OrderStatusCommandHandler( _mediator = mediator; _orderSettings = orderSettings; _loyaltyPointsSettings = loyaltyPointsSettings; + _logger = logger; } public async Task Handle(SetOrderStatusCommand request, CancellationToken cancellationToken) @@ -66,9 +70,17 @@ await _orderService.InsertOrderNote(new OrderNote { { //notification - var invoiceDownloadId = _orderSettings.AttachPdfInvoiceToOrderCompletedEmail - ? await _pdfService.SaveOrderToBinary(request.Order, "") - : null; + string invoiceDownloadId = null; + + try + { + if (_orderSettings.AttachPdfInvoiceToOrderCompletedEmail) + invoiceDownloadId = await _pdfService.SaveOrderToBinary(request.Order, ""); + } + catch (Exception ex) + { + _logger.LogError(ex, "Error - order completed attachment file {OrderOrderNumber}", request.Order.OrderNumber); + } await _orderMessageProviderService .SendOrderCompletedCustomerMessage(request.Order, customer, request.Order.CustomerLanguageId, diff --git a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs index 61077b01a6..9100e5c435 100644 --- a/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs +++ b/src/Business/Grand.Business.Checkout/Commands/Handlers/Orders/ProcessOrderPaidCommandHandler.cs @@ -7,6 +7,7 @@ using Grand.Domain.Localization; using Grand.Domain.Orders; using Grand.Mediator; +using Microsoft.Extensions.Logging; namespace Grand.Business.Checkout.Commands.Handlers.Orders; @@ -14,6 +15,7 @@ public class ProcessOrderPaidCommandHandler : IRequestHandler _logger; private readonly IMediator _mediator; private readonly IOrderMessageProviderService _orderMessageProviderService; private readonly OrderSettings _orderSettings; @@ -25,7 +27,8 @@ public ProcessOrderPaidCommandHandler( IMediator mediator, IPdfService pdfService, OrderSettings orderSettings, - LanguageSettings languageSettings) + LanguageSettings languageSettings, + ILogger logger) { _orderMessageProviderService = orderMessageProviderService; _customerService = customerService; @@ -33,6 +36,7 @@ public ProcessOrderPaidCommandHandler( _pdfService = pdfService; _orderSettings = orderSettings; _languageSettings = languageSettings; + _logger = logger; } public async Task Handle(ProcessOrderPaidCommand request, CancellationToken cancellationToken) @@ -61,9 +65,17 @@ protected virtual async Task ProcessOrderPaid(Order order) { var customer = await _customerService.GetCustomerById(order.CustomerId); - var invoiceDownloadId = _orderSettings.AttachPdfInvoiceToOrderPaidEmail - ? await _pdfService.SaveOrderToBinary(order, "") - : null; + string invoiceDownloadId = null; + + try + { + if (_orderSettings.AttachPdfInvoiceToOrderPaidEmail) + invoiceDownloadId = await _pdfService.SaveOrderToBinary(order, ""); + } + catch (Exception ex) + { + _logger.LogError(ex, "Error - order paid attachment file {OrderOrderNumber}", order.OrderNumber); + } await _orderMessageProviderService.SendOrderPaidCustomerMessage(order, customer, order.CustomerLanguageId, invoiceDownloadId); diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/MarkAsPaidCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/MarkAsPaidCommandHandlerTests.cs index 50f46b48b6..28616690d5 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/MarkAsPaidCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/MarkAsPaidCommandHandlerTests.cs @@ -11,6 +11,7 @@ using Grand.Domain.Orders; using Grand.Domain.Payments; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -65,7 +66,8 @@ public async Task Handle_OrderAwaitingApproval_StaysAwaitingAndRunsNoPaidProcess var checkStatus = new CheckOrderStatusCommandHandler(paidMediator.Object, _orderServiceMock.Object, new OrderSettings()); var paid = new ProcessOrderPaidCommandHandler(messageProvider.Object, customerService.Object, - paidMediator.Object, new Mock().Object, new OrderSettings(), new LanguageSettings()); + paidMediator.Object, new Mock().Object, new OrderSettings(), new LanguageSettings(), + new Mock>().Object); _mediatorMock.Setup(x => x.Send(It.IsAny(), default)) .ReturnsAsync(true); _mediatorMock.Setup(x => x.Send(It.IsAny(), default)) diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs index e90ea4f80a..653c4d68d6 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/OrderStatusCommandHandlerTests.cs @@ -7,6 +7,7 @@ using Grand.Domain.Customers; using Grand.Domain.Orders; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -39,7 +40,7 @@ public void Init() _handler = new OrderStatusCommandHandler(_orderServiceMock.Object, _customerServiceMock.Object, _pdfServiceMock.Object, _orderMessageProviderServiceMock.Object, _vendorServiceMock.Object, _mediatorMock.Object, - _orderSettings, _loyaltyPointsSettings); + _orderSettings, _loyaltyPointsSettings, new Mock>().Object); } [TestMethod] @@ -70,6 +71,25 @@ await _handler.Handle( "en", "invoice"), Times.Once); } + [TestMethod] + public async Task Handle_OrderCompleted_InvoiceCannotBeGenerated_StillSendsTheMessageAndAwardsLoyaltyPoints() + { + _orderSettings.AttachPdfInvoiceToOrderCompletedEmail = true; + _loyaltyPointsSettings.PointsForPurchases_Awarded = (int)OrderStatusSystem.Complete; + var order = new Order { OrderStatusId = (int)OrderStatusSystem.Pending, CustomerLanguageId = "en" }; + _pdfServiceMock.Setup(c => c.SaveOrderToBinary(order, It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException()); + + await _handler.Handle( + new SetOrderStatusCommand { Order = order, Os = OrderStatusSystem.Complete, NotifyCustomer = true }, + CancellationToken.None); + + _orderMessageProviderServiceMock.Verify(c => c.SendOrderCompletedCustomerMessage(order, It.IsAny(), + "en", null), Times.Once); + _mediatorMock.Verify(c => c.Send(It.IsAny(), It.IsAny()), + Times.Once); + } + [TestMethod] public async Task Handle_OrderCompleted_InvoiceDisabled_SavesNoInvoice() { diff --git a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs index 686026b93b..b3476b59de 100644 --- a/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs +++ b/src/Tests/Grand.Business.Checkout.Tests/Commands/Handlers/Orders/ProcessOrderPaidCommandHandlerTests.cs @@ -8,6 +8,7 @@ using Grand.Domain.Localization; using Grand.Domain.Orders; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -33,7 +34,8 @@ public void Init() _pdfServiceMock = new Mock(); _orderSettings = new OrderSettings(); _handler = new ProcessOrderPaidCommandHandler(_messageProviderMock.Object, _customerServiceMock.Object, - _mediatorMock.Object, _pdfServiceMock.Object, _orderSettings, new LanguageSettings()); + _mediatorMock.Object, _pdfServiceMock.Object, _orderSettings, new LanguageSettings(), + new Mock>().Object); } [TestMethod] @@ -77,4 +79,20 @@ public async Task Handle_InvoiceEnabled_AttachesTheSavedInvoice() _messageProviderMock.Verify(x => x.SendOrderPaidCustomerMessage(order, It.IsAny(), It.IsAny(), "invoice"), Times.Once); } + + [TestMethod] + public async Task Handle_InvoiceCannotBeGenerated_StillSendsThePaidMessages() + { + _orderSettings.AttachPdfInvoiceToOrderPaidEmail = true; + var order = new Order { OrderTotal = 10, OrderStatusId = (int)OrderStatusSystem.Pending }; + _pdfServiceMock.Setup(x => x.SaveOrderToBinary(order, It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException()); + + await _handler.Handle(new ProcessOrderPaidCommand { Order = order }, CancellationToken.None); + + _messageProviderMock.Verify(x => x.SendOrderPaidCustomerMessage(order, It.IsAny(), + It.IsAny(), null), Times.Once); + _messageProviderMock.Verify(x => x.SendOrderPaidStoreOwnerMessage(order, It.IsAny(), + It.IsAny()), Times.Once); + } } From 4020b40bc51d1c7c2063abe9c4694835ee7d53a1 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 18:53:02 +0100 Subject: [PATCH 3/6] Delete an invoice that no e-mail took over, also after an error --- .../Services/OrderMessageProviderService.cs | 36 +++++++++++++++---- .../Services/MessageProviderServiceTest.cs | 27 +++++++++++++- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs b/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs index 358a4cfa1e..f9575fd19e 100644 --- a/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs +++ b/src/Business/Grand.Business.Messages/Services/OrderMessageProviderService.cs @@ -11,6 +11,7 @@ using Grand.Domain.Stores; using Grand.Domain.Vendors; using Grand.Mediator; +using Microsoft.Extensions.Logging; namespace Grand.Business.Messages.Services; @@ -22,12 +23,14 @@ public OrderMessageProviderService( IMessageLookupService messageLookupService, IMessageProviderService messageProviderService, IDownloadService downloadService, - IMediator mediator) + IMediator mediator, + ILogger logger) { _messageLookupService = messageLookupService; _messageProviderService = messageProviderService; _downloadService = downloadService; _mediator = mediator; + _logger = logger; } #endregion @@ -38,6 +41,7 @@ public OrderMessageProviderService( private readonly IMessageProviderService _messageProviderService; private readonly IDownloadService _downloadService; private readonly IMediator _mediator; + private readonly ILogger _logger; #endregion @@ -225,14 +229,32 @@ public virtual async Task SendOrderRefundedCustomerMessage(Order order, dou private async Task SendOrderInvoiceMessage(string message, Order order, Customer customer, string languageId, string invoiceDownloadId, DomainHost host = null) { - var queuedEmails = await SendOrderCustomerMessage(message, order, customer, languageId, invoiceDownloadId, - host: host); + var queuedEmails = 0; + try + { + queuedEmails = await SendOrderCustomerMessage(message, order, customer, languageId, invoiceDownloadId, + host: host); + return queuedEmails; + } + finally + { + //inactive template, no recipient or an error - no email owns the invoice, so nothing would ever delete it + if (queuedEmails == 0) + await DeleteUnusedInvoice(invoiceDownloadId); + } + } - //inactive template or no recipient - no email owns the invoice, so nothing would ever delete it - if (queuedEmails == 0) + private async Task DeleteUnusedInvoice(string invoiceDownloadId) + { + try + { await _downloadService.DeleteDownloadById(invoiceDownloadId); - - return queuedEmails; + } + catch (Exception ex) + { + //a broken file must not stop the rest of the order processing + _logger.LogError(ex, "Error deleting the invoice download {InvoiceDownloadId}", invoiceDownloadId); + } } /// diff --git a/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs b/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs index 17fe8d526f..1cb66aa12c 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Services/MessageProviderServiceTest.cs @@ -18,6 +18,7 @@ using Grand.Domain.Stores; using Grand.Domain.Vendors; using Grand.Mediator; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -96,7 +97,7 @@ public void Init() _mediatorMock.Object, _commonSetting.Object); _downloadServiceMock = new Mock(); _orderMessageService = new OrderMessageProviderService(messageLookupService, _messageService, - _downloadServiceMock.Object, _mediatorMock.Object); + _downloadServiceMock.Object, _mediatorMock.Object, new Mock>().Object); _newsletterMessageService = new NewsletterMessageProviderService(messageLookupService, _messageService, _mediatorMock.Object); _merchandiseReturnMessageService = new MerchandiseReturnMessageProviderService(messageLookupService, @@ -193,6 +194,30 @@ public async Task SendOrderCompletedCustomerMessage_NoRecipient_DeletesTheInvoic _downloadServiceMock.Verify(x => x.DeleteDownloadById("invoice"), Times.Once); } + [TestMethod] + public async Task SendOrderPaidCustomerMessage_EmailCannotBeQueued_DeletesTheInvoiceAndRethrows() + { + _queuedEmailServiceMock.Setup(x => x.InsertQueuedEmail(It.IsAny())) + .ThrowsAsync(new InvalidOperationException()); + + await Assert.ThrowsExactlyAsync(() => + _orderMessageService.SendOrderPaidCustomerMessage(OrderWithBillingEmail("a@b.c"), new Customer(), "123", + "invoice")); + + _downloadServiceMock.Verify(x => x.DeleteDownloadById("invoice"), Times.Once); + } + + [TestMethod] + public async Task SendOrderPaidCustomerMessage_NoEmailAndInvoiceCannotBeDeleted_DoesNotThrow() + { + _downloadServiceMock.Setup(x => x.DeleteDownloadById("invoice")).ThrowsAsync(new InvalidOperationException()); + + var result = await _orderMessageService.SendOrderPaidCustomerMessage(OrderWithBillingEmail(null), + new Customer(), "123", "invoice"); + + Assert.AreEqual(0, result); + } + [TestMethod] public async Task SendNewsLetterSubscriptionActivationMessage_QueuesEmail() { From 99f2dd6fa4b819fc49547446ad42eb678b98f133 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 18:53:03 +0100 Subject: [PATCH 4/6] Delete invoice e-mails with the customer and invoices before their e-mails --- .ai/skills/message-notification.md | 2 +- .../Messages/IQueuedEmailService.cs | 6 +++ .../Events/CustomerDeletedEventHandler.cs | 27 ++++++++++++ .../Services/QueuedEmailService.cs | 32 +++++++++++--- .../CustomerDeletedEventHandlerTests.cs | 39 +++++++++++++++++ .../Services/QueuedEmailServiceTests.cs | 43 +++++++++++++++++++ .../Controllers/QueuedEmailController.cs | 2 + 7 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs create mode 100644 src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs diff --git a/.ai/skills/message-notification.md b/.ai/skills/message-notification.md index 1e35bdeb20..d3eacfb155 100644 --- a/.ai/skills/message-notification.md +++ b/.ai/skills/message-notification.md @@ -111,7 +111,7 @@ Task SendNotification( string invoiceDownloadId = null); ``` -`attachedDownloads` may be shared (a message template's attachment), so deleting an email leaves them alone. `invoiceDownloadId` is owned by that one email: `QueuedEmailService` deletes it with the email, and a requeued email gets its own copy. +`attachedDownloads` may be shared (a message template's attachment), so deleting an email leaves them alone. `invoiceDownloadId` is owned by that one email: `QueuedEmailService` deletes it with the email, and a requeued email gets its own copy. Deleting a customer deletes the invoice emails of that customer's orders, whatever address they went to. ### LiquidObjectBuilder (`src/Business/Grand.Business.Messages/DotLiquidDrops/LiquidObjectBuilder.cs`) ```csharp diff --git a/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs b/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs index f3bbe6870d..261588d9f1 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Messages/IQueuedEmailService.cs @@ -36,6 +36,12 @@ public interface IQueuedEmailService /// email Task DeleteCustomerEmail(string email); + /// + /// Deleted the invoice emails of orders and their invoices + /// + /// Order identifiers + Task DeleteOrderInvoiceEmails(IEnumerable orderIds); + /// /// Gets a queued email by identifier /// diff --git a/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs b/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs new file mode 100644 index 0000000000..95a8235b19 --- /dev/null +++ b/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs @@ -0,0 +1,27 @@ +using Grand.Business.Core.Interfaces.Checkout.Orders; +using Grand.Business.Core.Interfaces.Messages; +using Grand.Domain.Customers; +using Grand.Infrastructure.Events; +using Grand.Mediator; + +namespace Grand.Business.Messages.Events; + +public class CustomerDeletedEventHandler : INotificationHandler> +{ + private readonly IOrderService _orderService; + private readonly IQueuedEmailService _queuedEmailService; + + public CustomerDeletedEventHandler(IOrderService orderService, IQueuedEmailService queuedEmailService) + { + _orderService = orderService; + _queuedEmailService = queuedEmailService; + } + + public async Task Handle(EntityDeleted notification, CancellationToken cancellationToken) + { + //an invoice goes to the billing address of the order, which may not be the e-mail of the account + var orders = await _orderService.SearchOrders(customerId: notification.Entity.Id); + + await _queuedEmailService.DeleteOrderInvoiceEmails(orders.Select(x => x.Id)); + } +} diff --git a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs index 626908c575..c9c1e2ee83 100644 --- a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs +++ b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs @@ -72,10 +72,10 @@ public virtual async Task DeleteQueuedEmail(QueuedEmail queuedEmail) { ArgumentNullException.ThrowIfNull(queuedEmail); - await _queuedEmailRepository.DeleteAsync(queuedEmail); - await DeleteInvoices([queuedEmail.InvoiceDownloadId]); + await _queuedEmailRepository.DeleteAsync(queuedEmail); + //event notification await _mediator.EntityDeleted(queuedEmail); } @@ -114,9 +114,31 @@ public virtual async Task DeleteCustomerEmail(string email) var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( deleteCustomerEmail.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); + await DeleteInvoices(invoiceDownloadIds); + await _queuedEmailRepository.DeleteAsync(deleteCustomerEmail); + } + + /// + /// Deleted the invoice emails of orders and their invoices + /// + /// Order identifiers + public virtual async Task DeleteOrderInvoiceEmails(IEnumerable orderIds) + { + ArgumentNullException.ThrowIfNull(orderIds); + + var ids = orderIds.ToList(); + if (ids.Count == 0) + return; + + var invoiceEmails = _queuedEmailRepository.Table.Where(x => + x.Reference == Reference.Order && x.InvoiceDownloadId != null && ids.Contains(x.ObjectId)); + var invoiceDownloadIds = + await _queuedEmailRepository.ToListAsync(invoiceEmails.Select(x => x.InvoiceDownloadId)); await DeleteInvoices(invoiceDownloadIds); + + await _queuedEmailRepository.DeleteAsync(invoiceEmails); } /// @@ -223,9 +245,9 @@ public virtual async Task DeleteAllEmails() var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( _queuedEmailRepository.Table.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); - await _queuedEmailRepository.ClearAsync(); - await DeleteInvoices(invoiceDownloadIds); + + await _queuedEmailRepository.ClearAsync(); } private async Task DeleteInvoices(IEnumerable invoiceDownloadIds) @@ -237,7 +259,7 @@ private async Task DeleteInvoices(IEnumerable invoiceDownloadIds) } catch (Exception ex) { - //the emails are gone already; a broken file must not stop the rest nor the deletion of an account + //a broken file must not stop the rest nor the deletion of an email or an account _logger.LogError(ex, "Error deleting the invoice download {InvoiceDownloadId}", invoiceDownloadId); } } diff --git a/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs b/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs new file mode 100644 index 0000000000..ea60a002bc --- /dev/null +++ b/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs @@ -0,0 +1,39 @@ +using Grand.Business.Core.Interfaces.Checkout.Orders; +using Grand.Business.Core.Interfaces.Messages; +using Grand.Business.Messages.Events; +using Grand.Domain; +using Grand.Domain.Customers; +using Grand.Domain.Orders; +using Grand.Domain.Payments; +using Grand.Domain.Shipping; +using Grand.Infrastructure.Events; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Messages.Tests.Events; + +[TestClass] +public class CustomerDeletedEventHandlerTests +{ + [TestMethod] + public async Task Handle_DeletesTheInvoiceEmailsOfTheOrdersOfThatCustomer() + { + var customer = new Customer(); + var order = new Order { CustomerId = customer.Id }; + var orderService = new Mock(); + orderService.Setup(c => c.SearchOrders(It.IsAny(), It.IsAny(), customer.Id, It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .ReturnsAsync(new PagedList(new List { order }, 0, int.MaxValue)); + var queuedEmailService = new Mock(); + var handler = new CustomerDeletedEventHandler(orderService.Object, queuedEmailService.Object); + + await handler.Handle(new EntityDeleted(customer), CancellationToken.None); + + queuedEmailService.Verify(c => c.DeleteOrderInvoiceEmails( + It.Is>(ids => ids.Single() == order.Id)), Times.Once); + } +} diff --git a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs index b7ab8f6936..94816828c4 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs @@ -1,6 +1,7 @@ using Grand.Business.Core.Interfaces.Storage; using Grand.Business.Messages.Services; using Grand.Data; +using Grand.Domain.Common; using Grand.Domain.Media; using Grand.Domain.Messages; using Grand.Infrastructure.Events; @@ -166,6 +167,48 @@ public async Task DeleteAllEmails_DeletesEveryInvoice() _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Exactly(2)); } + [TestMethod] + public async Task DeleteAllEmails_DeletesTheInvoicesBeforeTheEmails() + { + SetupEmails(new QueuedEmail { InvoiceDownloadId = "invoice" }); + var calls = new List(); + _downloadServiceMock.Setup(c => c.DeleteDownloadById("invoice")).Callback(() => calls.Add("invoice")); + _repository.Setup(c => c.ClearAsync()).Callback(() => calls.Add("emails")); + + await _service.DeleteAllEmails(); + + CollectionAssert.AreEqual(new[] { "invoice", "emails" }, calls); + } + + [TestMethod] + public async Task DeleteOrderInvoiceEmails_DeletesOnlyTheInvoiceEmailsOfThoseOrders() + { + var invoiceEmail = new QueuedEmail { + To = "billing@b.c", Reference = Reference.Order, ObjectId = "order", InvoiceDownloadId = "mine" + }; + SetupEmails( + invoiceEmail, + new QueuedEmail { Reference = Reference.Order, ObjectId = "order" }, + new QueuedEmail { Reference = Reference.Order, ObjectId = "other", InvoiceDownloadId = "other" }); + IEnumerable deleted = null; + _repository.Setup(c => c.DeleteAsync(It.IsAny>())) + .Callback>(emails => deleted = emails.ToList()); + + await _service.DeleteOrderInvoiceEmails(["order"]); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById("mine"), Times.Once); + _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Once); + CollectionAssert.AreEqual(new[] { invoiceEmail }, deleted.ToArray()); + } + + [TestMethod] + public async Task DeleteOrderInvoiceEmails_NoOrders_DeletesNothing() + { + await _service.DeleteOrderInvoiceEmails([]); + + _repository.Verify(c => c.DeleteAsync(It.IsAny>()), Times.Never); + } + [TestMethod] public async Task CopyInvoiceDownload_CreatesASeparateDownload() { diff --git a/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs b/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs index 5f30efd408..a661d57fac 100644 --- a/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/QueuedEmailController.cs @@ -192,6 +192,8 @@ public async Task Requeue(QueuedEmailModel queuedEmailModel) AttachedDownloads = queuedEmail.AttachedDownloads, InvoiceDownloadId = await _queuedEmailService.CopyInvoiceDownload(queuedEmail.InvoiceDownloadId), EmailAccountId = queuedEmail.EmailAccountId, + Reference = queuedEmail.Reference, + ObjectId = queuedEmail.ObjectId, DontSendBeforeDateUtc = queuedEmailModel.SendImmediately || !queuedEmailModel.DontSendBeforeDate.HasValue ? null : _dateTimeService.ConvertToUtcTime(queuedEmailModel.DontSendBeforeDate.Value) From 5caba04c7d69e1ab90295e42c742310a5565f6e7 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 19:09:53 +0100 Subject: [PATCH 5/6] Keep invoices owned while e-mails are queued and deleted --- .../Services/QueuedEmailService.cs | 49 ++++++++----- .../Services/QueuedEmailServiceTests.cs | 70 +++++++++++++++++-- 2 files changed, 93 insertions(+), 26 deletions(-) diff --git a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs index c9c1e2ee83..c382e0fd9c 100644 --- a/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs +++ b/src/Business/Grand.Business.Messages/Services/QueuedEmailService.cs @@ -44,7 +44,16 @@ public virtual async Task InsertQueuedEmail(QueuedEmail queuedEmail) { ArgumentNullException.ThrowIfNull(queuedEmail); - await _queuedEmailRepository.InsertAsync(queuedEmail); + try + { + await _queuedEmailRepository.InsertAsync(queuedEmail); + } + catch + { + //the email was not stored, so nothing would ever delete its invoice + await DeleteInvoices([queuedEmail.InvoiceDownloadId]); + throw; + } //event notification await _mediator.EntityInserted(queuedEmail); @@ -110,13 +119,9 @@ public virtual async Task DeleteCustomerEmail(string email) { ArgumentNullException.ThrowIfNull(email); - var deleteCustomerEmail = _queuedEmailRepository.Table.Where(x => x.To == email); - var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( - deleteCustomerEmail.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); + await DeleteInvoiceEmails(_queuedEmailRepository.Table.Where(x => x.To == email)); - await DeleteInvoices(invoiceDownloadIds); - - await _queuedEmailRepository.DeleteAsync(deleteCustomerEmail); + await _queuedEmailRepository.DeleteManyAsync(x => x.To == email && x.InvoiceDownloadId == null); } /// @@ -131,14 +136,8 @@ public virtual async Task DeleteOrderInvoiceEmails(IEnumerable orderIds) if (ids.Count == 0) return; - var invoiceEmails = _queuedEmailRepository.Table.Where(x => - x.Reference == Reference.Order && x.InvoiceDownloadId != null && ids.Contains(x.ObjectId)); - var invoiceDownloadIds = - await _queuedEmailRepository.ToListAsync(invoiceEmails.Select(x => x.InvoiceDownloadId)); - - await DeleteInvoices(invoiceDownloadIds); - - await _queuedEmailRepository.DeleteAsync(invoiceEmails); + await DeleteInvoiceEmails(_queuedEmailRepository.Table.Where(x => + x.Reference == Reference.Order && ids.Contains(x.ObjectId))); } /// @@ -242,12 +241,24 @@ public virtual async Task> SearchEmails(string fromEmail /// public virtual async Task DeleteAllEmails() { - var invoiceDownloadIds = await _queuedEmailRepository.ToListAsync( - _queuedEmailRepository.Table.Where(x => x.InvoiceDownloadId != null).Select(x => x.InvoiceDownloadId)); + await DeleteInvoiceEmails(_queuedEmailRepository.Table); - await DeleteInvoices(invoiceDownloadIds); + await _queuedEmailRepository.DeleteManyAsync(x => x.InvoiceDownloadId == null); + } - await _queuedEmailRepository.ClearAsync(); + /// + /// Deletes the emails with an invoice, invoices first: once an email is gone, nothing leads to its invoice. + /// Only the emails read here are deleted, so one queued in the meantime keeps its invoice + /// + private async Task DeleteInvoiceEmails(IQueryable emails) + { + var invoiceEmails = await _queuedEmailRepository.ToListAsync(emails + .Where(x => x.InvoiceDownloadId != null) + .Select(x => new QueuedEmail { Id = x.Id, InvoiceDownloadId = x.InvoiceDownloadId })); + + await DeleteInvoices(invoiceEmails.Select(x => x.InvoiceDownloadId)); + + await _queuedEmailRepository.DeleteAsync(invoiceEmails); } private async Task DeleteInvoices(IEnumerable invoiceDownloadIds) diff --git a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs index 94816828c4..65012e1eec 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Services/QueuedEmailServiceTests.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using System.Linq.Expressions; namespace Grand.Business.Messages.Tests.Services; @@ -25,8 +26,9 @@ public class QueuedEmailServiceTests public void Init() { _repository = new Mock>(); - _repository.Setup(c => c.ToListAsync(It.IsAny>(), It.IsAny())) - .Returns((IQueryable query, CancellationToken _) => Task.FromResult>(query.ToList())); + _repository.Setup(c => c.ToListAsync(It.IsAny>(), It.IsAny())) + .Returns((IQueryable query, CancellationToken _) => + Task.FromResult>(query.ToList())); _downloadServiceMock = new Mock(); _mediatorMock = new Mock(); _loggerMock = new Mock>(); @@ -49,6 +51,29 @@ public async Task InsertQueuedEmail_ValidArgument_InvokeExpectedMethods() _mediatorMock.Verify(c => c.Publish(It.IsAny>(), default), Times.Once); } + [TestMethod] + public async Task InsertQueuedEmail_NotStored_DeletesTheInvoiceAndRethrows() + { + _repository.Setup(c => c.InsertAsync(It.IsAny())).ThrowsAsync(new InvalidOperationException()); + + await Assert.ThrowsExactlyAsync(() => + _service.InsertQueuedEmail(new QueuedEmail { InvoiceDownloadId = "invoice" })); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById("invoice"), Times.Once); + } + + [TestMethod] + public async Task InsertQueuedEmail_StoredButEventFails_KeepsTheInvoice() + { + _mediatorMock.Setup(c => c.Publish(It.IsAny>(), default)) + .ThrowsAsync(new InvalidOperationException()); + + await Assert.ThrowsExactlyAsync(() => + _service.InsertQueuedEmail(new QueuedEmail { InvoiceDownloadId = "invoice" })); + + _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Never); + } + [TestMethod] public void InsertQueuedEmail_NullArguemnt_ThrowException() { @@ -83,14 +108,45 @@ public async Task DeleteQueuedEmail_ValidArgument_InvokeExpectedMethods() public async Task DeleteCustomerEmail_ValidArgument_InvokeExpectedMethods() { await _service.DeleteCustomerEmail("email@email.com"); - _repository.Verify(c => c.DeleteAsync(It.IsAny>()), Times.Once); + _repository.Verify(c => c.DeleteManyAsync(It.IsAny>>()), Times.Once); + } + + [TestMethod] + public async Task DeleteCustomerEmail_AnEmailWithAnInvoiceIsLeftToTheInvoiceDeletion() + { + Expression> filter = null; + _repository.Setup(c => c.DeleteManyAsync(It.IsAny>>())) + .Callback>>(f => filter = f); + + await _service.DeleteCustomerEmail("a@b.c"); + + var matches = filter.Compile(); + Assert.IsTrue(matches(new QueuedEmail { To = "a@b.c" })); + Assert.IsFalse(matches(new QueuedEmail { To = "other@b.c" })); + //queued after the invoices were read, so its invoice was not deleted + Assert.IsFalse(matches(new QueuedEmail { To = "a@b.c", InvoiceDownloadId = "queued-meanwhile" })); } [TestMethod] public async Task DeleteAllEmails_ValidArgument_InvokeExpectedMethods() { await _service.DeleteAllEmails(); - _repository.Verify(c => c.ClearAsync(), Times.Once); + _repository.Verify(c => c.DeleteManyAsync(It.IsAny>>()), Times.Once); + } + + [TestMethod] + public async Task DeleteAllEmails_AnEmailWithAnInvoiceIsLeftToTheInvoiceDeletion() + { + Expression> filter = null; + _repository.Setup(c => c.DeleteManyAsync(It.IsAny>>())) + .Callback>>(f => filter = f); + + await _service.DeleteAllEmails(); + + var matches = filter.Compile(); + Assert.IsTrue(matches(new QueuedEmail())); + //queued after the invoices were read, so its invoice was not deleted + Assert.IsFalse(matches(new QueuedEmail { InvoiceDownloadId = "queued-meanwhile" })); } [TestMethod] @@ -161,7 +217,6 @@ public async Task DeleteAllEmails_DeletesEveryInvoice() await _service.DeleteAllEmails(); - _repository.Verify(c => c.ClearAsync(), Times.Once); _downloadServiceMock.Verify(c => c.DeleteDownloadById("first"), Times.Once); _downloadServiceMock.Verify(c => c.DeleteDownloadById("second"), Times.Once); _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Exactly(2)); @@ -173,7 +228,8 @@ public async Task DeleteAllEmails_DeletesTheInvoicesBeforeTheEmails() SetupEmails(new QueuedEmail { InvoiceDownloadId = "invoice" }); var calls = new List(); _downloadServiceMock.Setup(c => c.DeleteDownloadById("invoice")).Callback(() => calls.Add("invoice")); - _repository.Setup(c => c.ClearAsync()).Callback(() => calls.Add("emails")); + _repository.Setup(c => c.DeleteAsync(It.IsAny>())) + .Callback(() => calls.Add("emails")); await _service.DeleteAllEmails(); @@ -198,7 +254,7 @@ public async Task DeleteOrderInvoiceEmails_DeletesOnlyTheInvoiceEmailsOfThoseOrd _downloadServiceMock.Verify(c => c.DeleteDownloadById("mine"), Times.Once); _downloadServiceMock.Verify(c => c.DeleteDownloadById(It.IsAny()), Times.Once); - CollectionAssert.AreEqual(new[] { invoiceEmail }, deleted.ToArray()); + CollectionAssert.AreEqual(new[] { invoiceEmail.Id }, deleted.Select(x => x.Id).ToArray()); } [TestMethod] From c2ee2f17c3295939991ab0097d775ce18a114198 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 19:26:01 +0100 Subject: [PATCH 6/6] Find a deleted customer's orders by id, deleted orders included --- .../Events/CustomerDeletedEventHandler.cs | 18 ++++++++----- .../CustomerDeletedEventHandlerTests.cs | 27 ++++++++----------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs b/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs index 95a8235b19..5944b5b916 100644 --- a/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs +++ b/src/Business/Grand.Business.Messages/Events/CustomerDeletedEventHandler.cs @@ -1,6 +1,7 @@ -using Grand.Business.Core.Interfaces.Checkout.Orders; using Grand.Business.Core.Interfaces.Messages; +using Grand.Data; using Grand.Domain.Customers; +using Grand.Domain.Orders; using Grand.Infrastructure.Events; using Grand.Mediator; @@ -8,20 +9,23 @@ namespace Grand.Business.Messages.Events; public class CustomerDeletedEventHandler : INotificationHandler> { - private readonly IOrderService _orderService; + private readonly IRepository _orderRepository; private readonly IQueuedEmailService _queuedEmailService; - public CustomerDeletedEventHandler(IOrderService orderService, IQueuedEmailService queuedEmailService) + public CustomerDeletedEventHandler(IRepository orderRepository, IQueuedEmailService queuedEmailService) { - _orderService = orderService; + _orderRepository = orderRepository; _queuedEmailService = queuedEmailService; } public async Task Handle(EntityDeleted notification, CancellationToken cancellationToken) { - //an invoice goes to the billing address of the order, which may not be the e-mail of the account - var orders = await _orderService.SearchOrders(customerId: notification.Entity.Id); + //an invoice goes to the billing address of the order, which may not be the e-mail of the account; + //a deleted order counts too, as its invoice email stays in the queue + var customerId = notification.Entity.Id; + var orderIds = await _orderRepository.ToListAsync( + _orderRepository.Table.Where(x => x.CustomerId == customerId).Select(x => x.Id), cancellationToken); - await _queuedEmailService.DeleteOrderInvoiceEmails(orders.Select(x => x.Id)); + await _queuedEmailService.DeleteOrderInvoiceEmails(orderIds); } } diff --git a/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs b/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs index ea60a002bc..dff535ba18 100644 --- a/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs +++ b/src/Tests/Grand.Business.Messages.Tests/Events/CustomerDeletedEventHandlerTests.cs @@ -1,11 +1,8 @@ -using Grand.Business.Core.Interfaces.Checkout.Orders; using Grand.Business.Core.Interfaces.Messages; using Grand.Business.Messages.Events; -using Grand.Domain; +using Grand.Data; using Grand.Domain.Customers; using Grand.Domain.Orders; -using Grand.Domain.Payments; -using Grand.Domain.Shipping; using Grand.Infrastructure.Events; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -16,24 +13,22 @@ namespace Grand.Business.Messages.Tests.Events; public class CustomerDeletedEventHandlerTests { [TestMethod] - public async Task Handle_DeletesTheInvoiceEmailsOfTheOrdersOfThatCustomer() + public async Task Handle_DeletesTheInvoiceEmailsOfTheOrdersOfThatCustomer_DeletedOrdersToo() { var customer = new Customer(); var order = new Order { CustomerId = customer.Id }; - var orderService = new Mock(); - orderService.Setup(c => c.SearchOrders(It.IsAny(), It.IsAny(), customer.Id, It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .ReturnsAsync(new PagedList(new List { order }, 0, int.MaxValue)); + var deletedOrder = new Order { CustomerId = customer.Id, Deleted = true }; + var otherOrder = new Order { CustomerId = "other" }; + var orderRepository = new Mock>(); + orderRepository.Setup(c => c.Table).Returns(new[] { order, deletedOrder, otherOrder }.AsQueryable()); + orderRepository.Setup(c => c.ToListAsync(It.IsAny>(), It.IsAny())) + .Returns((IQueryable query, CancellationToken _) => Task.FromResult>(query.ToList())); var queuedEmailService = new Mock(); - var handler = new CustomerDeletedEventHandler(orderService.Object, queuedEmailService.Object); + var handler = new CustomerDeletedEventHandler(orderRepository.Object, queuedEmailService.Object); await handler.Handle(new EntityDeleted(customer), CancellationToken.None); - queuedEmailService.Verify(c => c.DeleteOrderInvoiceEmails( - It.Is>(ids => ids.Single() == order.Id)), Times.Once); + queuedEmailService.Verify(c => c.DeleteOrderInvoiceEmails(It.Is>(ids => + ids.Count() == 2 && ids.Contains(order.Id) && ids.Contains(deletedOrder.Id))), Times.Once); } }