From bd6f43d80e470f862b0ea57ce6894ce0ac4aae65 Mon Sep 17 00:00:00 2001 From: support Date: Wed, 4 Nov 2026 19:58:30 +0100 Subject: [PATCH] Keep bulk order export and delete within the staff member's orders ExportExcelSelected and DeleteSelected took the order ids from the request and acted on every order found. They now skip orders the staff member's scope does not cover, as PdfInvoiceSelected already does, so a Sales Manager exports and deletes only their sales employees' orders. --- .../Controllers/OrderControllerScopeTests.cs | 85 +++++++++++++++++++ .../Controllers/OrderController.cs | 15 +++- 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 src/Tests/Grand.Web.Admin.Tests/Controllers/OrderControllerScopeTests.cs diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/OrderControllerScopeTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/OrderControllerScopeTests.cs new file mode 100644 index 000000000..c479a94af --- /dev/null +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/OrderControllerScopeTests.cs @@ -0,0 +1,85 @@ +using Grand.Business.Core.Commands.Checkout.Orders; +using Grand.Business.Core.Interfaces.Checkout.Orders; +using Grand.Business.Core.Interfaces.Checkout.Shipping; +using Grand.Business.Core.Interfaces.Common.Pdf; +using Grand.Business.Core.Interfaces.ExportImport; +using Grand.Domain.Orders; +using Grand.Domain.Shipping; +using Grand.Infrastructure; +using Grand.Mediator; +using Grand.Web.Admin.Controllers; +using Grand.Web.AdminShared.Interfaces; +using Grand.Web.Common.Localization; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Admin.Tests.Controllers; + +[TestClass] +public class OrderControllerScopeTests +{ + private readonly Order _allowed = new() { Id = "o1" }; + private readonly Order _denied = new() { Id = "o2" }; + private OrderController _controller; + private Mock> _exportManagerMock; + private Mock _mediatorMock; + private Mock _shipmentServiceMock; + + [TestInitialize] + public void Setup() + { + //GetOrdersByIds returns whatever the request names; the scope decides what the staff member may touch + var orderServiceMock = new Mock(); + orderServiceMock.Setup(s => s.GetOrdersByIds(It.IsAny())) + .ReturnsAsync((IList)new List { _allowed, _denied }); + var scopeMock = new Mock>(); + scopeMock.Setup(s => s.HasAccess(_allowed)).ReturnsAsync(true); + scopeMock.Setup(s => s.HasAccess(_denied)).ReturnsAsync(false); + + _exportManagerMock = new Mock>(); + _exportManagerMock.Setup(e => e.Export(It.IsAny>(), It.IsAny())) + .ReturnsAsync([]); + _mediatorMock = new Mock(); + _shipmentServiceMock = new Mock(); + _shipmentServiceMock.Setup(s => s.GetShipmentsByOrder(It.IsAny())) + .ReturnsAsync((IList)new List()); + + _controller = new OrderController( + new Mock().Object, + orderServiceMock.Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + _mediatorMock.Object, + scopeMock.Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + _exportManagerMock.Object) { + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; + } + + [TestMethod] + public async Task ExportExcelSelected_ScopeDenies_ExcludesOrderFromExport() + { + await _controller.ExportExcelSelected("o1,o2"); + + _exportManagerMock.Verify(e => e.Export(It.Is>( + orders => orders.Count() == 1 && orders.First() == _allowed), It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task DeleteSelected_ScopeDenies_DoesNotDeleteOrder() + { + await _controller.DeleteSelected(["o1", "o2"], _shipmentServiceMock.Object); + + _mediatorMock.Verify(m => m.Send(It.Is(c => c.Order == _allowed), + It.IsAny()), Times.Once); + _mediatorMock.Verify(m => m.Send(It.Is(c => c.Order == _denied), + It.IsAny()), Times.Never); + } +} diff --git a/src/Web/Grand.Web.Admin/Controllers/OrderController.cs b/src/Web/Grand.Web.Admin/Controllers/OrderController.cs index 07c86d668..ed1d9d325 100644 --- a/src/Web/Grand.Web.Admin/Controllers/OrderController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/OrderController.cs @@ -71,7 +71,7 @@ public async Task ExportExcelSelected(string selectedIds) if (selectedIds != null) { var ids = selectedIds.Split([','], StringSplitOptions.RemoveEmptyEntries).ToArray(); - orders.AddRange(await OrderService.GetOrdersByIds(ids)); + orders.AddRange(await GetAccessibleOrders(ids)); } var bytes = await exportManager.Export(orders, HttpContext.RequestAborted); @@ -87,7 +87,7 @@ public async Task DeleteSelected( if (selectedIds != null) { var orders = new List(); - orders.AddRange(await OrderService.GetOrdersByIds(selectedIds.ToArray())); + orders.AddRange(await GetAccessibleOrders(selectedIds.ToArray())); foreach (var order in orders) { var shipments = await shipmentService.GetShipmentsByOrder(order.Id); @@ -100,4 +100,15 @@ public async Task DeleteSelected( return Json(new { Result = true }); } + + //the ids come from the request and GetOrdersByIds does not filter them, so an order outside the + //staff member's scope (a Sales Manager's other sales employees) is skipped, as in PdfInvoiceSelected + private async Task> GetAccessibleOrders(string[] ids) + { + var orders = new List(); + foreach (var order in await OrderService.GetOrdersByIds(ids)) + if (await Scope.HasAccess(order)) + orders.Add(order); + return orders; + } }