From 0efa7b64809c48bbd3e212fb73a2ca0f54c5e725 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sat, 12 Sep 2026 15:15:08 +0200 Subject: [PATCH] Keep every store with an available currency A store whose published currencies are all limited to other stores made WorkingCurrency throw "No currency could be loaded" from ContextMiddleware, which took down every request for that store. A store manager could reach that state alone: unset the default currency, then unassign each remaining store-limited currency. Fall back to the primary store currency instead of throwing, and refuse the edits that would empty a store in the first place - in the store panel when unassigning the last available currency, and in Main Admin when saving store mappings that would leave any store without one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GrGY1NnCzdkn7wUxbyHzsX --- .../Services/CurrencyViewModelServiceTests.cs | 68 +++++++++++++ .../Controllers/CurrencyControllerTests.cs | 94 ++++++++++++++++++ .../Controllers/CurrencyController.cs | 9 ++ .../Interfaces/ICurrencyViewModelService.cs | 1 + .../Services/CurrencyViewModelService.cs | 31 ++++++ src/Web/Grand.Web.Common/WorkContextSetter.cs | 6 ++ .../Controllers/CurrencyController.cs | 5 + .../App_Data/Resources/DefaultLanguage.xml | Bin 1543710 -> 1544402 bytes .../App_Data/Resources/Upgrade/en_240.xml | 6 ++ 9 files changed, 220 insertions(+) create mode 100644 src/Tests/Grand.Web.Store.Tests/Controllers/CurrencyControllerTests.cs diff --git a/src/Tests/Grand.Web.Admin.Tests/Services/CurrencyViewModelServiceTests.cs b/src/Tests/Grand.Web.Admin.Tests/Services/CurrencyViewModelServiceTests.cs index 9e4caa617c..e0504913a3 100644 --- a/src/Tests/Grand.Web.Admin.Tests/Services/CurrencyViewModelServiceTests.cs +++ b/src/Tests/Grand.Web.Admin.Tests/Services/CurrencyViewModelServiceTests.cs @@ -1,8 +1,11 @@ using Grand.Business.Core.Interfaces.Common.Configuration; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; +using Grand.Business.Core.Interfaces.Common.Stores; using Grand.Domain.Directory; +using Grand.Domain.Stores; using Grand.Infrastructure.Caching; +using Grand.Web.AdminShared.Models.Directory; using Grand.Web.AdminShared.Services; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -14,6 +17,7 @@ namespace Grand.Web.Admin.Tests.Services; public class CurrencyViewModelServiceTests { private Mock _currencyServiceMock; + private Mock _storeServiceMock; private CurrencySettings _currencySettings; private Mock _settingServiceMock; private Mock _translationServiceMock; @@ -24,6 +28,7 @@ public class CurrencyViewModelServiceTests public void Setup() { _currencyServiceMock = new Mock(); + _storeServiceMock = new Mock(); _currencySettings = new CurrencySettings(); _settingServiceMock = new Mock(); _translationServiceMock = new Mock(); @@ -31,6 +36,7 @@ public void Setup() _service = new CurrencyViewModelService( _currencyServiceMock.Object, + _storeServiceMock.Object, _currencySettings, _settingServiceMock.Object, _translationServiceMock.Object, @@ -90,6 +96,68 @@ public async Task ValidateCurrencyUnpublish_OtherCurrenciesExist_CanProceed() Assert.IsTrue(canProceed); } + [TestMethod] + public async Task ValidateCurrencyStoreMapping_PublishedAndNotLimited_CanProceed() + { + var (canProceed, message) = await _service.ValidateCurrencyStoreMapping(new Currency { Id = "currency-1" }, + new CurrencyModel { Published = true, Stores = [] }); + + Assert.IsTrue(canProceed); + Assert.AreEqual(string.Empty, message); + _storeServiceMock.Verify(s => s.GetAllStores(), Times.Never); + } + + [TestMethod] + public async Task ValidateCurrencyStoreMapping_StoreLeftWithoutCurrency_CannotProceed() + { + _currencyServiceMock.Setup(c => c.GetAllCurrencies(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List { + new() { Id = "currency-1" }, + new() { Id = "currency-2", LimitedToStores = true, Stores = ["store-1"] } + }); + _storeServiceMock.Setup(s => s.GetAllStores()) + .ReturnsAsync(new List { new() { Id = "store-1" }, new() { Id = "store-2", Name = "Second" } }); + _translationServiceMock.Setup(t => t.GetResource("Admin.Configuration.Currencies.CantLimitStores")) + .Returns("Store '{0}' has no currency"); + + var (canProceed, message) = await _service.ValidateCurrencyStoreMapping(new Currency { Id = "currency-1" }, + new CurrencyModel { Published = true, Stores = ["store-1"] }); + + Assert.IsFalse(canProceed); + Assert.AreEqual("Store 'Second' has no currency", message); + } + + [TestMethod] + public async Task ValidateCurrencyStoreMapping_AnotherGlobalCurrencyExists_CanProceed() + { + _currencyServiceMock.Setup(c => c.GetAllCurrencies(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List { new() { Id = "currency-1" }, new() { Id = "currency-2" } }); + _storeServiceMock.Setup(s => s.GetAllStores()) + .ReturnsAsync(new List { new() { Id = "store-1" }, new() { Id = "store-2" } }); + + var (canProceed, _) = await _service.ValidateCurrencyStoreMapping(new Currency { Id = "currency-1" }, + new CurrencyModel { Published = true, Stores = ["store-1"] }); + + Assert.IsTrue(canProceed); + } + + [TestMethod] + public async Task ValidateCurrencyStoreMapping_UnpublishingLastCurrencyOfStore_CannotProceed() + { + _currencyServiceMock.Setup(c => c.GetAllCurrencies(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List { new() { Id = "currency-1" } }); + _storeServiceMock.Setup(s => s.GetAllStores()) + .ReturnsAsync(new List { new() { Id = "store-1", Name = "First" } }); + _translationServiceMock.Setup(t => t.GetResource("Admin.Configuration.Currencies.CantLimitStores")) + .Returns("Store '{0}' has no currency"); + + var (canProceed, message) = await _service.ValidateCurrencyStoreMapping(new Currency { Id = "currency-1" }, + new CurrencyModel { Published = false, Stores = [] }); + + Assert.IsFalse(canProceed); + Assert.AreEqual("Store 'First' has no currency", message); + } + [TestMethod] public async Task ValidateCurrencyDelete_PrimaryStoreCurrency_CannotDelete() { diff --git a/src/Tests/Grand.Web.Store.Tests/Controllers/CurrencyControllerTests.cs b/src/Tests/Grand.Web.Store.Tests/Controllers/CurrencyControllerTests.cs new file mode 100644 index 0000000000..6850d38479 --- /dev/null +++ b/src/Tests/Grand.Web.Store.Tests/Controllers/CurrencyControllerTests.cs @@ -0,0 +1,94 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Common.Localization; +using Grand.Business.Core.Interfaces.Common.Stores; +using Grand.Domain.Customers; +using Grand.Domain.Directory; +using Grand.Domain.Stores; +using Grand.Infrastructure; +using Grand.Web.Store.Controllers; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ViewFeatures; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using DomainStore = Grand.Domain.Stores.Store; + +namespace Grand.Web.Store.Tests.Controllers; + +[TestClass] +public class CurrencyControllerTests +{ + private const string StoreId = "storeId"; + + private CurrencyController _controller; + private Mock _currencyServiceMock; + private Mock _storeServiceMock; + private Mock _translationServiceMock; + private CurrencySettings _currencySettings; + + [TestInitialize] + public void Setup() + { + _currencyServiceMock = new Mock(); + _storeServiceMock = new Mock(); + _translationServiceMock = new Mock(); + _translationServiceMock.Setup(t => t.GetResource(It.IsAny())).Returns(x => x); + _currencySettings = new CurrencySettings(); + + var workContextMock = new Mock(); + workContextMock.Setup(w => w.CurrentCustomer).Returns(new Customer { StaffStoreId = StoreId }); + var contextAccessorMock = new Mock(); + contextAccessorMock.Setup(c => c.WorkContext).Returns(workContextMock.Object); + + _controller = new CurrencyController( + _currencyServiceMock.Object, + _currencySettings, + _translationServiceMock.Object, + _storeServiceMock.Object, + contextAccessorMock.Object); + + var httpContext = new DefaultHttpContext(); + _controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + _controller.TempData = new TempDataDictionary(httpContext, new Mock().Object); + } + + private static Currency StoreCurrency(string id) + { + return new Currency { Id = id, Published = true, LimitedToStores = true, Stores = [StoreId] }; + } + + [TestMethod] + public async Task UnassignStore_LastAvailableCurrency_IsRejected() + { + var currency = StoreCurrency("currency-1"); + _currencyServiceMock.Setup(c => c.GetCurrencyById(currency.Id)).ReturnsAsync(currency); + _storeServiceMock.Setup(s => s.GetStoreById(StoreId)).ReturnsAsync(new DomainStore { Id = StoreId }); + _currencyServiceMock.Setup(c => c.GetAllCurrencies(It.IsAny(), StoreId)) + .ReturnsAsync(new List { currency }); + + var result = await _controller.UnassignStore(currency.Id) as JsonResult; + + Assert.IsNotNull(result); + Assert.AreEqual("Admin.Configuration.Currencies.CantUnassignLast", + result.Value.GetType().GetProperty("message")!.GetValue(result.Value)); + Assert.IsTrue(currency.Stores.Contains(StoreId)); + _currencyServiceMock.Verify(c => c.UpdateCurrency(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task UnassignStore_AnotherCurrencyRemains_IsUnassigned() + { + var currency = StoreCurrency("currency-1"); + _currencyServiceMock.Setup(c => c.GetCurrencyById(currency.Id)).ReturnsAsync(currency); + _storeServiceMock.Setup(s => s.GetStoreById(StoreId)).ReturnsAsync(new DomainStore { Id = StoreId }); + _currencyServiceMock.Setup(c => c.GetAllCurrencies(It.IsAny(), StoreId)) + .ReturnsAsync(new List { currency, StoreCurrency("currency-2") }); + + var result = await _controller.UnassignStore(currency.Id) as JsonResult; + + Assert.IsNotNull(result); + Assert.IsTrue((bool)result.Value.GetType().GetProperty("success")!.GetValue(result.Value)!); + Assert.IsFalse(currency.Stores.Contains(StoreId)); + _currencyServiceMock.Verify(c => c.UpdateCurrency(currency), Times.Once); + } +} diff --git a/src/Web/Grand.Web.Admin/Controllers/CurrencyController.cs b/src/Web/Grand.Web.Admin/Controllers/CurrencyController.cs index f6630714b7..ac62df16c4 100644 --- a/src/Web/Grand.Web.Admin/Controllers/CurrencyController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/CurrencyController.cs @@ -225,6 +225,15 @@ public async Task Edit(CurrencyModel model, bool continueEditing) return RedirectToAction("Edit", new { id = currency.Id }); } + //ensure no store is left without an available currency + var (canMap, mappingMessage) = + await _currencyViewModelService.ValidateCurrencyStoreMapping(currency, model); + if (!canMap) + { + Error(mappingMessage); + return RedirectToAction("Edit", new { id = currency.Id }); + } + currency = await _currencyViewModelService.UpdateCurrencyModel(currency, model); Success(_translationService.GetResource("Admin.Configuration.Currencies.Updated")); if (continueEditing) diff --git a/src/Web/Grand.Web.AdminShared/Interfaces/ICurrencyViewModelService.cs b/src/Web/Grand.Web.AdminShared/Interfaces/ICurrencyViewModelService.cs index 4cabc67061..3fadf012ab 100644 --- a/src/Web/Grand.Web.AdminShared/Interfaces/ICurrencyViewModelService.cs +++ b/src/Web/Grand.Web.AdminShared/Interfaces/ICurrencyViewModelService.cs @@ -11,5 +11,6 @@ public interface ICurrencyViewModelService Task MarkAsPrimaryExchangeRateCurrency(string id); Task MarkAsPrimaryStoreCurrency(string id); Task<(bool canProceed, string message)> ValidateCurrencyUnpublish(string currencyId, bool published); + Task<(bool canProceed, string message)> ValidateCurrencyStoreMapping(Currency currency, CurrencyModel model); Task<(bool canDelete, string message)> ValidateCurrencyDelete(Currency currency); } \ No newline at end of file diff --git a/src/Web/Grand.Web.AdminShared/Services/CurrencyViewModelService.cs b/src/Web/Grand.Web.AdminShared/Services/CurrencyViewModelService.cs index d33f6092d8..9e9c5f2bc7 100644 --- a/src/Web/Grand.Web.AdminShared/Services/CurrencyViewModelService.cs +++ b/src/Web/Grand.Web.AdminShared/Services/CurrencyViewModelService.cs @@ -1,6 +1,7 @@ using Grand.Business.Core.Interfaces.Common.Configuration; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; +using Grand.Business.Core.Interfaces.Common.Stores; using Grand.Domain.Directory; using Grand.Infrastructure.Caching; using Grand.Web.AdminShared.Extensions.Mapping; @@ -14,6 +15,7 @@ public class CurrencyViewModelService : ICurrencyViewModelService #region Fields private readonly ICurrencyService _currencyService; + private readonly IStoreService _storeService; private readonly CurrencySettings _currencySettings; private readonly ISettingService _settingService; private readonly ITranslationService _translationService; @@ -24,12 +26,14 @@ public class CurrencyViewModelService : ICurrencyViewModelService #region Constructors public CurrencyViewModelService(ICurrencyService currencyService, + IStoreService storeService, CurrencySettings currencySettings, ISettingService settingService, ITranslationService translationService, ICacheBase cacheBase) { _currencyService = currencyService; + _storeService = storeService; _currencySettings = currencySettings; _settingService = settingService; _translationService = translationService; @@ -90,6 +94,33 @@ public virtual async Task MarkAsPrimaryStoreCurrency(string id) return (true, string.Empty); } + public virtual async Task<(bool canProceed, string message)> ValidateCurrencyStoreMapping(Currency currency, + CurrencyModel model) + { + var limitedToStores = model.Stores is { Length: > 0 }; + + //a published currency available to every store can never leave a store without one + if (model.Published && !limitedToStores) + return (true, string.Empty); + + var otherCurrencies = (await _currencyService.GetAllCurrencies()) + .Where(c => c.Id != currency.Id).ToList(); + + foreach (var store in await _storeService.GetAllStores()) + { + if (otherCurrencies.Any(c => !c.LimitedToStores || c.Stores.Contains(store.Id))) + continue; + + if (model.Published && (!limitedToStores || model.Stores.Contains(store.Id))) + continue; + + return (false, string.Format( + _translationService.GetResource("Admin.Configuration.Currencies.CantLimitStores"), store.Name)); + } + + return (true, string.Empty); + } + public virtual async Task<(bool canDelete, string message)> ValidateCurrencyDelete(Currency currency) { if (currency.Id == _currencySettings.PrimaryStoreCurrencyId) diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index 805ace465f..7c32b24a1f 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -397,6 +397,12 @@ protected async Task WorkingCurrency(Customer customer, Language langu var allStoreCurrencies = await _currencyService.GetAllCurrencies(storeId: store.Id); + //no published currency is mapped to the store - fall back to the primary store currency + //rather than failing every request for that store + if (!allStoreCurrencies.Any()) + return await _currencyService.GetPrimaryStoreCurrency() ?? + throw new Exception("No currency could be loaded"); + if (allStoreCurrencies.Count == 1) return allStoreCurrencies.FirstOrDefault(); diff --git a/src/Web/Grand.Web.Store/Controllers/CurrencyController.cs b/src/Web/Grand.Web.Store/Controllers/CurrencyController.cs index 3004601f1c..494e23b131 100644 --- a/src/Web/Grand.Web.Store/Controllers/CurrencyController.cs +++ b/src/Web/Grand.Web.Store/Controllers/CurrencyController.cs @@ -108,6 +108,11 @@ public async Task UnassignStore(string id) if (store?.DefaultCurrencyId == currency.Id) return Json(new { success = false, message = translationService.GetResource("Admin.Configuration.Currencies.CantUnassignDefault") }); + //the store must keep at least one available currency, otherwise its work context cannot be built + var storeCurrencies = await currencyService.GetAllCurrencies(storeId: storeId); + if (!storeCurrencies.Any(c => c.Id != currency.Id)) + return Json(new { success = false, message = translationService.GetResource("Admin.Configuration.Currencies.CantUnassignLast") }); + if (currency.Stores.Remove(storeId)) await currencyService.UpdateCurrency(currency); diff --git a/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml b/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml index 09de03d11a7f4f0e600c8da30ba5a3a32ed83502..38ceab54f4cdb014dbdcfdb7a043671950e65b82 100644 GIT binary patch delta 266 zcmbO?C+^b1xP}(S7N!>F7M3ln9*&$j44DkMKwL7t@F=6>^b4oiL@d=Asu>IzY8liS z6d1}G@)=4Qau`y8>?DR%AejRs(-=y?azK3y8DJG4e&Y0t^Mu5wyFBI>njX^0S~ESs znpJGOgCna4<8&Y?F@4ioA&Kb+f*IAOe^6(VnC=EtD4Yv4z8Gv=5zydVhD3%;h%p;> z@^MYSvzB!N*qHVXCsrV417da{<^W<&Am##MZXo6XVqPHT17dz4764*FAQl2*VIUR( PVo@L#+uq?MKFu2d6{Arg delta 127 zcmcb#FmB$QxP}(S7N!>F7M3ln9*)yrIkO5(SDVI~HvPd9exd0B_N+qFH%w!lFg?JU zRjl2{l@*BDfS4VKIe?fGh`E568;E&;m=}oofS4bM1%Ox(h=qVy7>GrHSQLoGw%fRh HPxA%<#5^bX diff --git a/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_240.xml b/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_240.xml index 9f51c3550a..175f8def6d 100644 --- a/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_240.xml +++ b/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_240.xml @@ -81,9 +81,15 @@ Global currencies cannot be modified by store owners. + + The store '{0}' would be left without an available currency. + The default store currency can't be unassigned. + + At least one currency must remain available for your store. + This currency is not assigned to your store.